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

Add a glance_metric component #4452

Merged
merged 2 commits into from
Nov 29, 2024
Merged

Add a glance_metric component #4452

merged 2 commits into from
Nov 29, 2024

Conversation

pezholio
Copy link
Contributor

@pezholio pezholio commented Nov 28, 2024

This is based on the glance_metric component used in content-data-admin with some unused options removed. I’ve also tweaked the display of the explicit label, as aria-label is not supported on non-semantic elements,

Screenshot

image

Trello card: https://trello.com/c/eJTfFnFN/700-create-rollup-for-view-page-and-review-page

@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4452 November 28, 2024 17:24 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4452 November 28, 2024 17:26 Inactive
@pezholio pezholio force-pushed the add-glance-metric-component branch from 1cace6c to 55dc5a7 Compare November 28, 2024 17:34
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4452 November 28, 2024 17:34 Inactive
Copy link
Contributor

@AshGDS AshGDS left a comment

Choose a reason for hiding this comment

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

Looks good, added some comments.

@pezholio pezholio force-pushed the add-glance-metric-component branch from 55dc5a7 to 68a8147 Compare November 29, 2024 14:17
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4452 November 29, 2024 14:17 Inactive
@pezholio pezholio requested a review from AshGDS November 29, 2024 14:18
@pezholio pezholio force-pushed the add-glance-metric-component branch from 68a8147 to bac6f4c Compare November 29, 2024 14:22
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4452 November 29, 2024 14:22 Inactive
@pezholio pezholio force-pushed the add-glance-metric-component branch from bac6f4c to d25ff76 Compare November 29, 2024 14:34
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4452 November 29, 2024 14:34 Inactive
Copy link
Contributor

@AshGDS AshGDS left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Just needs one small change but will approve so I'm not a blocker for it.

- communicate number value and label (if present) in a single dictation when read with a screen reader
- convey the meaning of the number shown
- The component must use the correct heading level for the page (defaults to `<h3>`)

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add uses_component_wrapper_helper: true so the functionality of it is surfaced in the docs? Thanks

This is based on the `glance_metric` component used in
[content-data-admin](https://github.com/alphagov/content-data-admin)
with some unused options removed. I’ve also tweaked the display of the
explicit label, as `aria-label` is not supported on non-semantic
elements,
@pezholio pezholio force-pushed the add-glance-metric-component branch from d25ff76 to d4f4dd8 Compare November 29, 2024 14:56
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4452 November 29, 2024 14:57 Inactive
@pezholio pezholio merged commit 90d6577 into main Nov 29, 2024
12 checks passed
@pezholio pezholio deleted the add-glance-metric-component branch November 29, 2024 15:00
pezholio added a commit that referenced this pull request Dec 2, 2024
* Use component wrapper on panel component ([PR #4459](#4459))
* Use the component wrapper on the phase banner component ([PR #4460](#4460))
* Add a `glance_metric` component ([PR #4452](#4452))
* Use component wrapper on org logo component ([PR #4458](#4458))
* Use component wrapper on notice component ([PR #4444](#4444))
* Add component wrapper to the metadata component ([PR #4442](#4442))
@pezholio pezholio mentioned this pull request Dec 2, 2024
pezholio added a commit that referenced this pull request Dec 2, 2024
* Use component wrapper on panel component ([PR #4459](#4459))
* Use the component wrapper on the phase banner component ([PR #4460](#4460))
* Add a `glance_metric` component ([PR #4452](#4452))
* Use component wrapper on org logo component ([PR #4458](#4458))
* Use component wrapper on notice component ([PR #4444](#4444))
* Add component wrapper to the metadata component ([PR #4442](#4442))
pezholio added a commit that referenced this pull request Dec 2, 2024
* Use component wrapper on panel component ([PR #4459](#4459))
* Use the component wrapper on the phase banner component ([PR #4460](#4460))
* Add a `glance_metric` component ([PR #4452](#4452))
* Use component wrapper on org logo component ([PR #4458](#4458))
* Use component wrapper on notice component ([PR #4444](#4444))
* Add component wrapper to the metadata component ([PR #4442](#4442))
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