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.

Sign up for GitHub

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

Jump to bottom

feat(filter-panel): add view all button for filter groups #8188

Merged
merged 25 commits into from
Feb 10, 2022
Merged

feat(filter-panel): add view all button for filter groups #8188

merged 25 commits into from
Feb 10, 2022

Conversation

jkaeser
Copy link
Member

@jkaeser jkaeser commented Feb 1, 2022 •
edited
Loading

Related Ticket(s)

Resolves #7405

Description

Adds support for "view all" buttons in DDSFilterGroupItem components.

  • The button text defaults to "View all" and can be changed with the view-all-text attribute.
  • The button appears when the number of filters in the group exceeds the max-filters attribute value. This defaults to 7, as per the functional specs.
  • When the button appears, the number of filters specified by the filter-cutoff attribute are shown at first (defaults to 5 as per functional specs). The hidden filters are revealed once the button has been clicked.
  • The shown/hidden filters are reset once the filter group is toggled closed and then re-opened.
    • There is one exception to this: if one of the filters that would be hidden has been selected, all filter items are revealed when re-opening the filter group.

Changelog

New

  • "View all" buttons render in DDSFilterGroupItem components when a sufficient number of filters are present in the group.

Changed

  • Split the DDSFilterPanelComposite's modal and desktop rendering into two methods to make it more obvious what's going on.

@jkaeser jkaeser force-pushed the feat/filter-panel-view-all-7405 branch from a29f4d3 to 21f26e9 Compare February 1, 2022 21:36
@jkaeser jkaeser marked this pull request as ready for review February 1, 2022 21:53
@jkaeser jkaeser requested a review from a team as a code owner February 1, 2022 21:53
@jkaeser jkaeser requested a review from emyarod February 1, 2022 21:53
@jkaeser jkaeser added 👀 eyes needed package: web components Work necessary for the IBM.com Library web components package labels Feb 1, 2022
@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Feb 1, 2022 •
edited
Loading

Deploy preview created for package Web Components HTML Test Application:
https://ibmdotcom-web-components-html-test-upstream.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/8188/index.html

Built with commit: 11535762c04f4d38659b49ce822db2c755e0bce0

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Feb 1, 2022 •
edited
Loading

Deploy preview created for package Web Components:
https://ibmdotcom-webcomponents.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/8188/index.html

Built with commit: 11535762c04f4d38659b49ce822db2c755e0bce0

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Feb 1, 2022 •
edited
Loading

Deploy preview created for package React:
https://ibmdotcom-react.s3.us-south.cloud-object-storage.appdomain.cloud/deploy-previews/8188/index.html

Built with commit: 11535762c04f4d38659b49ce822db2c755e0bce0

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Feb 2, 2022 •
edited
Loading

Deploy preview created for package "React (Codesandbox Examples)":
https://react-codesandbox.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/8188/index.html

Built with commit: 11535762c04f4d38659b49ce822db2c755e0bce0

@proeung proeung added the Needs design approval PRs on feature requests and new components have to get design approval before merge. label Feb 2, 2022
@oliviaflory oliviaflory mentioned this pull request Feb 2, 2022
Closed
@oliviaflory oliviaflory self-assigned this Feb 2, 2022
@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Feb 2, 2022 •
edited
Loading

Deploy preview created for package NextJS Test Application:
https://ibmdotcom-nextjs-test-upstream.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/8188/index.html

Built with commit: 11535762c04f4d38659b49ce822db2c755e0bce0

@oliviaflory
Copy link
Contributor

Hi @jkaeser overall this is looking really good!

  1. Keyboard focus lost after selecting View all
    We are losing the keyboard focus after selecting the view all button, in this recording selecting the view all loses the focus and the next tab click goes to the Technologies accordion. I think the focus should be placed on the first revealed filter item, in this case the 5th item Hardware.
    filter panel view all losing focus Feb-02-2022 16-17-47

  2. View all button
    View all button should be following the Carbon ghost button colors, I think we're missing the text color changing to $hover-primary-text token on hover. Carbon ghost button style documentation

Screen Shot 2022-02-02 at 4 28 48 PM

  1. Question

When the button appears, the number of filters specified by the filter-cutoff attribute are shown at first (defaults to 5 as per functional specs). The hidden filters are revealed once the button has been clicked.

This seems great, does it apply to the whole filter panel, ie every filter group would have the same number (5) OR can the author/developer customize each filter group to a unique filter-cutoff attribute, some 5, some 7?

@proeung
Copy link
Contributor

proeung commented Feb 8, 2022

@oliviaflory @IgnacioBecerra Can we get a re-review? Thanks!

@oliviaflory
Copy link
Contributor

@jkaeser @proeung It looks like the web components deploy preview and code sandbox failed? I'm seeing the rounded focus state on the View all button still.

@IgnacioBecerra IgnacioBecerra mentioned this pull request Feb 8, 2022
Closed
IgnacioBecerra
IgnacioBecerra approved these changes Feb 8, 2022
View reviewed changes
Copy link
Contributor

@IgnacioBecerra IgnacioBecerra left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@jkaeser LGTM, this component can be complicated but you were able to implement the feature, thanks for the amazing job working on this!

Focus looks as expected 👍
Screen Shot 2022-02-08 at 3 00 45 PM

@proeung
Copy link
Contributor

proeung commented Feb 8, 2022

@jkaeser @proeung It looks like the web components deploy preview and code sandbox failed? I'm seeing the rounded focus state on the View all button still.

@oliviaflory Yeah, I'm seeing the rounded focus state styling on the Sandbox preview as well. This is odd because the SB preview looks correct (see attached). I wonder if there's any other style import that we're missing on the Sandbox app side. @IgnacioBecerra let me know what you think.

Screen Shot 2022-02-08 at 6 07 39 PM

@IgnacioBecerra
Copy link
Contributor

IgnacioBecerra commented Feb 8, 2022 •
edited
Loading

@proeung The deploy preview WC CodeSandbox hasn't been updated since 5 days ago due to a CI error that was fixed in the latest commits, so it could be that it's still building. Although it could also be due to some Jenkins problems we ran into this morning and the changes aren't reflecting in the sandbox just yet.

But since the style changes were done in the regular scss file the FilterPanel gets all its styles from, there's no missing imports to be added. Seems like we just gotta wait for the sandbox preview to be updated

@proeung
Copy link
Contributor

proeung commented Feb 9, 2022

@IgnacioBecerra Makes sense! I just checked the latest WC sandbox deploy preview and the rounded focus style is no longer there. I think we're good to mark this PR as ready for merge. @oliviaflory Let me know if you agree.

Screen Shot 2022-02-09 at 2 09 14 PM
https://webcomponents-codesandbox.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/8188/filter-panel/index.html

oliviaflory
oliviaflory approved these changes Feb 9, 2022
View reviewed changes
Copy link
Contributor

@oliviaflory oliviaflory left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @jkaeser 🎉

@proeung proeung added Ready to merge Label for the pull requests that are ready to merge and removed 👀 eyes needed labels Feb 9, 2022
@oliviaflory oliviaflory removed the Needs design approval PRs on feature requests and new components have to get design approval before merge. label Feb 9, 2022
@kodiakhq kodiakhq bot merged commit 6a55992 into carbon-design-system:main Feb 10, 2022
IgnacioBecerra added a commit to IgnacioBecerra/ibm-dotcom-library that referenced this pull request Feb 11, 2022
@IgnacioBecerra
Revert "feat(filter-panel): add view all button for filter groups (ca…
8bcc2e1 
…rbon-design-system#8188)"

This reverts commit 6a55992.
@jeffchew jeffchew mentioned this pull request Feb 11, 2022
Closed
2 tasks
@IgnacioBecerra IgnacioBecerra mentioned this pull request Feb 11, 2022
Merged
kodiakhq bot pushed a commit that referenced this pull request Feb 11, 2022
@IgnacioBecerra
Revert "feat(filter-panel): add view all button for filter groups" (#…
97aa93b 
…8252)

This reverts commit 6a55992.

### Related Ticket(s)
Reverting #8188 

### Description
Reverting for the time being.

### Changelog

**Removed**

- the changes done in the linked PR

<!-- React and Web Component deploy previews are enabled by default. -->
<!-- To enable additional available deploy previews, apply the following -->
<!-- labels for the corresponding package: -->
<!-- *** "package: services": Services -->
<!-- *** "package: utilities": Utilities -->
<!-- *** "RTL": React / Web Components (RTL) -->
<!-- *** "feature flag": React / Web Components (experimental) -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliviaflory oliviaflory oliviaflory approved these changes

@IgnacioBecerra IgnacioBecerra IgnacioBecerra approved these changes

@proeung proeung proeung approved these changes

@emyarod emyarod Awaiting requested review from emyarod emyarod is a code owner automatically assigned from carbon-design-system/carbon-for-ibm-com-members

@RichKummer RichKummer Awaiting requested review from RichKummer

Assignees

@oliviaflory oliviaflory

Labels
package: web components Work necessary for the IBM.com Library web components package Ready to merge Label for the pull requests that are ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Filter panel] Web Component: add View all link for filter groups - DDS Consulting
7 participants
@jkaeser @ibmdotcom-bot @oliviaflory @proeung @IgnacioBecerra @emyarod @jeffchew