fix(status-bar): use static toggle button text instead of dynamic expand/collapse text#2012
fix(status-bar): use static toggle button text instead of dynamic expand/collapse text#2012akashsonune wants to merge 1 commit intomainfrom
Conversation
akashsonune
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new toggleButtonText input to the SiStatusBarComponent to consolidate the expand and collapse button labels, marking the previous individual inputs as deprecated. The review identifies a need to update the documentation to refer to the component as a 'status bar' rather than a 'navbar' for clarity. Additionally, the reviewer highlights that the change is breaking and requires explicit documentation in the PR footer according to the repository's contribution guidelines.
| */ | ||
| readonly blinkPulse = input<Observable<boolean>>(); | ||
| /** | ||
| * Text for the navbar expand/collapse toggle button. Required for a11y. |
There was a problem hiding this comment.
The documentation refers to a "navbar", but this component is the SiStatusBarComponent. Please update this to "status bar" to ensure accuracy and clarity.
| * Text for the navbar expand/collapse toggle button. Required for a11y. | |
| * Text for the status bar expand/collapse toggle button. Required for a11y. |
References
- When naming UI elements, prioritize consistency with related components to maintain a consistent user experience across related features.
| * t(() => $localize`:@@SI_STATUS_BAR.TOGGLE:Toggle`) | ||
| * ``` | ||
| */ | ||
| readonly toggleButtonText = input(t(() => $localize`:@@SI_STATUS_BAR.TOGGLE:Toggle`)); |
There was a problem hiding this comment.
The introduction of toggleButtonText and the removal of expandButtonText/collapseButtonText from the template is a breaking change. Any custom values previously provided to the deprecated inputs will now be ignored, and the button label will default to "Toggle".
According to the repository style guide (line 232), breaking changes must be documented in the PR description with a BREAKING CHANGE: footer including summary, description, and migration instructions. Additionally, consider if a fallback mechanism is needed to maintain backward compatibility for existing customizations during the deprecation period.
References
- Breaking changes must include BREAKING CHANGE: in the footer with summary, description, and migration instructions. (link)
…and/collapse text
akashsonune
force-pushed
the
fix/a11y-status-bar-toggle-button-text
branch
from
7ba3c38 to
9c01475
Compare
1 participant
…and/collapse text
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: