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

fix: Properly create new rows in filtered views #1409

Merged
merged 2 commits into from
Feb 19, 2025
Merged

fix: Properly create new rows in filtered views #1409

merged 2 commits into from
Feb 19, 2025

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Oct 8, 2024 •
edited
Loading

First attempt to improve #638 by applying exact view filters as a default value for new rows

  • Apply filter values as defaults where feasible
  • Have some tests

Follow up

  • Warn during view creation if a new view leads to issues (e.g. a column is hidden but no defaults are present or can be assumed)

@juliusknorr juliusknorr added bug Something isn't working 2. developing Work in progress feature: views labels Oct 9, 2024
@juliusknorr juliusknorr force-pushed the fix/638 branch 2 times, most recently from 39ae5ec to 2feba05 Compare February 7, 2025 07:53
@juliusknorr juliusknorr added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Feb 7, 2025
@juliusknorr
Copy link
Member Author

This is good from my side as a first iteration.

Tried to come up with some test cases as well.

@juliusknorr juliusknorr marked this pull request as ready for review February 7, 2025 11:39
@juliusknorr juliusknorr requested a review from a team February 12, 2025 21:32
blizzz
blizzz approved these changes Feb 13, 2025
View reviewed changes
Copy link
Member

@blizzz blizzz 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.

Looks good, great with tests!

Just CI a bit needy

@juliusknorr juliusknorr force-pushed the fix/638 branch 2 times, most recently from 0e79701 to 994dfb0 Compare February 13, 2025 20:56
@juliusknorr juliusknorr force-pushed the fix/638 branch 2 times, most recently from 6745592 to 730e1c2 Compare February 14, 2025 00:02
@blizzz
Copy link
Member

blizzz commented Feb 14, 2025

failing test is unrelated and fixed with #1612

@blizzz blizzz merged commit 14c65c1 into main Feb 19, 2025
63 checks passed
@blizzz blizzz deleted the fix/638 branch February 19, 2025 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blizzz blizzz blizzz approved these changes

@enjeck enjeck Awaiting requested review from enjeck enjeck is a code owner

Assignees
No one assigned
Labels
3. to review Waiting for reviews bug Something isn't working feature: views
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@juliusknorr @blizzz