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

fix: (Datagrid) filters fix #5878

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

xmannyxfreshx1
Copy link
Contributor

Closes #4538

What did you change?

  • This replaces the use of localDispatch inside applyFilters() with local state. Functionally, it's still applying those filters when isFetching is true.
  • Also, isFetching is now simulated when using the refresh control in the filter storybook examples.

How did you test and verify your work?

Storybook

@xmannyxfreshx1 xmannyxfreshx1 requested a review from a team as a code owner August 15, 2024 20:42
@xmannyxfreshx1 xmannyxfreshx1 requested review from kennylam and anamikaanu96 and removed request for a team August 15, 2024 20:42
Copy link
Contributor

github-actions bot commented Aug 15, 2024

DCO Assistant Lite bot All contributors have signed the DCO.

Copy link

netlify bot commented Aug 15, 2024

Deploy Preview for carbon-for-ibm-products ready!

Name Link
🔨 Latest commit 2783f30
🔍 Latest deploy log https://app.netlify.com/sites/carbon-for-ibm-products/deploys/66e338c6602b280008a6f3c0
😎 Deploy Preview https://deploy-preview-5878--carbon-for-ibm-products.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@xmannyxfreshx1
Copy link
Contributor Author

I have read the DCO document and I hereby sign the DCO.

@xmannyxfreshx1 xmannyxfreshx1 changed the title Datagrid filters fix fix: (Datagrid) filters fix Aug 15, 2024
@anamikaanu96
Copy link
Contributor

Hi @xmannyxfreshx1 Thank you for your contribution!

Looks like this issue is addressed on this PR. Can you please confirm.

@xmannyxfreshx1
Copy link
Contributor Author

Hi, @anamikaanu96 just took a look at that PR and yep it appears that it resolves the issue this PR was trying to fix. We can close both this PR and the tagged issue. Thanks

@xmannyxfreshx1
Copy link
Contributor Author

The only other thing I would request is if we could add these changes
Screenshot 2024-09-19 at 11 41 50 AM
to the filtering storybook examples. These changes reflect a more real world usage of the refresh button where the isFetching state is changed.

@amal-k-joy
Copy link
Contributor

The only other thing I would request is if we could add these changes Screenshot 2024-09-19 at 11 41 50 AM to the filtering storybook examples. These changes reflect a more real world usage of the refresh button where the isFetching state is changed.

Hey @xmannyxfreshx1 ,
Thanks for confirming that the original issue is fixed.

There seems to be a console error for other stories where setIsFetching is not passed and used batch actions.
I have feeling like isFetching can be managed by the adopter itself.
@davidmenendez , can you share your thoughts on this ?
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Datagrid] Filter panel selection issues
3 participants