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

refactor: remove useless _filterValue property and related listeners #7284

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

vursen
Copy link
Contributor

@vursen vursen commented Mar 28, 2024

Description

The PR removes the _filterValue property along with its related listeners from <vaadin-grid-filter-column />. This property seems to be useless, as the grid listens for filter-changed directly on <vaadin-grid-filter />.

Type of change

  • Refactor

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@vursen
Copy link
Contributor Author

vursen commented Mar 28, 2024

The only purpose of this property I can think of is to keep the filter value in case the grid clears the cell content and calls the renderer again. However, this cannot happen because the grid filter column doesn't support custom renderers.

Copy link
Member

@web-padawan web-padawan left a comment

Choose a reason for hiding this comment

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

LGTM. This logic was added when moving from <template> to renderer functions in #1967. Looks like it's not necessary since #4978 where we added SlotController handling the slotted vaadin-text-field and adding listener to it.

@vursen vursen merged commit d6c485b into main Mar 28, 2024
9 checks passed
@vursen vursen deleted the refactor/grid/remove-unused-filter-value-property branch March 28, 2024 11:58
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.4.0.alpha20 and is also targeting the upcoming stable 24.4.0 version.

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.

3 participants