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

Allow disable widgets filtering #268

Merged

Conversation

Clebal
Copy link
Contributor

@Clebal Clebal commented Jan 7, 2022

Story details: https://app.shortcut.com/cartoteam/story/193004

Added property filterable in every widget with filtering capabilities.

When filterable is false, widget's click event is disabled.

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #193004: Disable interactivity for widgets.

@Clebal Clebal changed the title Disable interactivity for widgets Disable filtering for widgets Jan 7, 2022
@coveralls
Copy link
Collaborator

coveralls commented Jan 7, 2022

Pull Request Test Coverage Report for Build 1668486363

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 70.314%

Totals Coverage Status
Change from base Build 1649446088: 0.01%
Covered Lines: 1142
Relevant Lines: 1529

💛 - Coveralls

@Clebal Clebal changed the title Disable filtering for widgets Allow disable widgets filtering Jan 7, 2022
Copy link
Contributor

@eamador eamador left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@VictorVelarde VictorVelarde left a comment

Choose a reason for hiding this comment

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

Code LGTM, nice addition @Clebal

Just a question: to land this (for an alpha) we can do it without updating docs & without any example on carto-react-template, but I think it would be nice to create also those other 2 small companion PRs, is that possible?.

If not, we can do that later, when making official 1.2 release... thoughts ? cc/ @borja-munoz

@Clebal
Copy link
Contributor Author

Clebal commented Jan 11, 2022

@VictorVelarde totally agree. But I cannot tackle that right now since I don't have specific dedication for C4R, I'm sorry. I can make it in a future, maybe for the final release.

@borja-munoz
Copy link
Contributor

In my opinion, the carto-react-template example is not mandatory in this case, but updating the documentation (adding the property to the reference) is a must.

@VictorVelarde
Copy link
Contributor

I've created this task for doc (https://app.shortcut.com/cartoteam/story/202069/widgets-document-new-filterable-prop-for-widgets), but I think we can already land this one

Copy link
Contributor

@VictorVelarde VictorVelarde left a comment

Choose a reason for hiding this comment

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

🚀

@VictorVelarde VictorVelarde merged commit 362c4d3 into master Jan 12, 2022
@VictorVelarde VictorVelarde deleted the feature/sc-193004/disable-interactivity-for-widgets branch January 12, 2022 15:27
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.

5 participants