Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WAITP-1287: Dashboard dropdown #4655

Merged
merged 8 commits into from
Jun 19, 2023

Conversation

tcaiger
Copy link
Contributor

@tcaiger tcaiger commented Jun 19, 2023

Issue #: WAITP-1287: Dashboard dropdown

Changes:

  • Tweak DashboardRoute to return correct dashboards and to return response in camel case
  • Add Dashboards query
  • Add dashboard menu component which creates a menu of links to specific dashboards

Screenshots:

image

@tcaiger tcaiger changed the title Waitp 1287 dashboard dropdown WAITP-1287: Dashboard dropdown Jun 19, 2023
Copy link
Contributor

@alexd-bes alexd-bes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking really good. I had a couple of questions, but happy to pre-approve.

display: flex;
justify-content: space-between;
align-items: center;
background: ${PANEL_GREY};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of putting this grey in the theme? I'm guessing that if we ever themed the app for a particular organisation then this colour may potentially be a part of that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes that's a good idea. We will have to come up with a scheme to make all these theme colors make sense at some point. Perhaps it will need to evolve over time. I've added a panel section to the theme and used that through out the side bar for now.

any,
any
>;
export type DashboardsRequest = Request<any, any, any, any>;

export class DashboardsRoute extends Route<DashboardsRequest> {
public async buildResponse() {
const { query, ctx } = this.req;
const { organisationUnitCode, projectCode } = query;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were thinking about removing organisationUnitCode in favour of entityCode, right? What do you think of doing the web-server side of it now, so that eventually it just slots right into the entity server as and when changes are made? I know it's not strictly part of this ticket, but something to consider I guess?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acdunham The back end needs a bit of a clean up anyway so I reckon if the front end looks good let's not let the back end hold it up

Comment on lines +28 to +30
const dashboards = await ctx.services.central.fetchResources('dashboards', {
filter: { root_entity_code: baseEntity.code },
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A dashboard attached to an entity can also appear on any of that entity's children (depending on the existence of dashboard items which are usually specific to entity types) which is why I was checking against ancestors.

This route handler needs another fix-up pass anyway so I'm happy to pick that up in a separate PR

@tcaiger tcaiger merged commit 94489d5 into epic-frontend-rewrite Jun 19, 2023
@tcaiger tcaiger deleted the waitp-1287-dashboard-dropdown branch June 19, 2023 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants