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 configuration for icon mappings and spinner mappings to show/hide their text within a table's cells #2012

Merged
merged 21 commits into from
Apr 17, 2024

Conversation

mollykreis
Copy link
Contributor

Pull Request

🀨 Rationale

This updates the nimble-mapping-icon and nimble-mapping-spinner elements to be aligned with the changes described in #1992.

πŸ‘©β€πŸ’» Implementation

  • Added text-hidden to nimble-mapping-icon and nimble-mapping-spinner
    • I also added these to Blazor and Angular so that when the change is published clients in any framework can uptake the new package and have the opportunity to add text-hidden to mappings in order to keep their existing behavior
  • Reworked the MappingIconConfig and MappingSpinnerConfig to be more aligned
    • They both create the appropriate ViewTemplate to render a spinner, icon, or blank
    • They both add the appropriate title and aria-* attributes based on the configuration
    • Neither adds css classes that are styled in the group header styles or cell view styles
  • Update the templates for the icon column's cell view and group header share code for rendering icons and spinners (enabled because of aligning MappingIconConfig and MappingSpinnerConfig) and to render the text based on the text-hidden attribute

πŸ§ͺ Testing

  • Updated storybook & matrix tests
  • Manually tested
  • Wrote/updated a number of tests for the icon column
    • To enable this, created a page object for the icon column to encapsulate the logic around finding the icon/spinner and the text

βœ… Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

@mollykreis
Copy link
Contributor Author

@atmgrifter00, will you buddy this PR for me?

@mollykreis mollykreis marked this pull request as ready for review April 12, 2024 15:12
@mollykreis mollykreis merged commit 6d50261 into main Apr 17, 2024
13 checks passed
@mollykreis mollykreis deleted the label-hidden-mappings branch April 17, 2024 20:39
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.

6 participants