Skip to content

Align chart toolbar icon buttons and unify their disabled state - #3035

Open
khairul-syazwan wants to merge 3 commits into
developfrom
khairul-syazwan/analyze-2929
Open

Align chart toolbar icon buttons and unify their disabled state#3035
khairul-syazwan wants to merge 3 commits into
developfrom
khairul-syazwan/analyze-2929

Conversation

@khairul-syazwan

Copy link
Copy Markdown
Collaborator

Closes #2929

Download and drilldown disabled
Before:
2026-08-03T00-00-00Z-before-download-and-drilldown-disabled
After:
2026-08-03T00-00-00Z-after-download-and-drilldown-disabled

Hover on disabled drilldown
Before:
2026-08-03T00-00-00Z-before-hover-on-disabled-drilldown
After:
2026-08-03T00-00-00Z-after-hover-on-disabled-drilldown

Toolbar on default chart
Before:
2026-08-03T00-00-00Z-before-toolbar-default-chart
After:
2026-08-03T00-00-00Z-after-toolbar-default-chart

Toolbar on Patient List
Before:
2026-08-03T00-00-00Z-before-toolbar-patient-list
After:
2026-08-03T00-00-00Z-after-toolbar-patient-list

Each icon button in the Patient Analytics chart toolbar action group brought its own box, icon size and disabled treatment, so the row never lined up and Download rendered lighter than Filter by Selection when both were disabled.

The download button is a toolbar button nested inside a Bootstrap dropdown trigger, so Bootstrap's .btn:disabled { opacity: .65 } stacked on top of the button's own opacity: .6, giving an effective 0.39 against 0.6 for every other disabled button in the group.

  • Add mriToolbarIconButton and mriToolbarIconButtonDisabled mixins so the chart, drilldown, filter summary and download buttons share one box, radius, icon size, glyph alignment and disabled treatment.
  • Mark the disabled state with --color-mri-disabled-text alone instead of the token plus a per-button opacity that Bootstrap could compound.
  • Suppress the hover mixin on disabled buttons, which used to turn the glyph near-white on a transparent background.
  • Flatten the Bootstrap dropdown trigger wrapping the download button so it contributes no box, colour, focus ring or disabled opacity of its own.

Scoped to .chartToolbar-main-container .actionButtonGroup .iconActionButton, leaving the other toolbarButton and actionButton users unchanged.

Merge Checklist

Please cross check this list if additions / modifications needs to be done on top of your core changes and tick them off. Reviewer can as well glance through and help the developer if something is missed out.

  • Automated Tests (Jasmine integration tests, Unit tests, and/or Performance tests)
  • Updated Manual tests / Demo Config
  • Documentation (Application guide, Admin guide, Markdown, Readme and/or Wiki)
  • Verified that local development environment is working with latest changes (integrated with latest develop branch)
  • following best practices in code review doc

Closes #2929

Each icon button in the Patient Analytics chart toolbar action group brought
its own box, icon size and disabled treatment, so the row never lined up and
Download rendered lighter than Filter by Selection when both were disabled.

The download button is a toolbar button nested inside a Bootstrap dropdown
trigger, so Bootstrap's `.btn:disabled { opacity: .65 }` stacked on top of the
button's own `opacity: .6`, giving an effective 0.39 against 0.6 for every
other disabled button in the group.

- Add `mriToolbarIconButton` and `mriToolbarIconButtonDisabled` mixins so the
  chart, drilldown, filter summary and download buttons share one box, radius,
  icon size, glyph alignment and disabled treatment.
- Mark the disabled state with `--color-mri-disabled-text` alone instead of the
  token plus a per-button opacity that Bootstrap could compound.
- Suppress the hover mixin on disabled buttons, which used to turn the glyph
  near-white on a transparent background.
- Flatten the Bootstrap dropdown trigger wrapping the download button so it
  contributes no box, colour, focus ring or disabled opacity of its own.

Scoped to `.chartToolbar-main-container .actionButtonGroup .iconActionButton`,
leaving the other `toolbarButton` and `actionButton` users unchanged.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

The new dropdown-trigger styling removes the only visible focus indicator for the Download control without providing a replacement, creating a keyboard accessibility regression.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR aligns the visual layout and disabled-state behavior of the Patient Analytics chart toolbar icon buttons by introducing shared SCSS mixins and neutralizing Bootstrap dropdown-trigger styling that previously compounded disabled opacity for the Download control.

Changes:

  • Added shared mriToolbarIconButton / mriToolbarIconButtonDisabled mixins to unify icon-button geometry (box, radius, icon sizing/alignment) and disabled styling.
  • Updated chart toolbar styling to apply the shared mixins to chart/drilldown/filter/download buttons and to standardize disabled treatment without stacked opacity.
  • Flattened the Bootstrap dropdown trigger wrapper around the Download button to prevent it from contributing its own box styling and disabled opacity.
File summaries
File Description
plugins/ui/apps/vue-mri-ui-lib/src/styles/chartToolbar.scss Applies the new shared icon-button styling within the chart toolbar action group and neutralizes Bootstrap dropdown trigger styling for Download.
plugins/ui/apps/vue-mri-ui-lib/src/styles/_mixins.scss Introduces shared mixins for consistent icon-button geometry and a single disabled treatment across chart toolbar actions.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread plugins/ui/apps/vue-mri-ui-lib/src/styles/chartToolbar.scss
@khairul-syazwan khairul-syazwan changed the title Flign chart toolbar icon buttons and unify their disabled state Align chart toolbar icon buttons and unify their disabled state Aug 3, 2026
The flattened bs-dropdown trigger and the shared icon-button mixin set
outline/box-shadow to none without a replacement, leaving keyboard users
with no visible focus indicator. Add a :focus-visible ring using the
existing --color-focus token to all toolbar icon buttons and the
dropdown trigger.
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.

[Patient List] - Disable behaviour for "Download" and "Filter by selection" is different on Patient List view

3 participants