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 slice change when filters are present #4098

Merged
merged 1 commit into from Feb 26, 2024
Merged

Conversation

benjaminpkane
Copy link
Contributor

What changes are proposed in this pull request?

Fixes changing the group grid slice when a filter is present. Optional chaining was missing. See recording

Before

Screen.Recording.2024-02-21.at.11.30.39.AM.mov

After

Screen.Recording.2024-02-21.at.11.30.20.AM.mov

How is this patch tested? If it is not, please explain why.

Locally

Release Notes

  • Fixed changing group slice in the grid when a sidebar filter is present

What areas of FiftyOne does this PR affect?

  • App: FiftyOne application changes
  • Build: Build and test infrastructure changes
  • Core: Core fiftyone Python library changes
  • Documentation: FiftyOne documentation changes
  • Other

@benjaminpkane benjaminpkane added bug Bug fixes app Issues related to App features labels Feb 21, 2024
@benjaminpkane benjaminpkane self-assigned this Feb 21, 2024
Copy link

codecov bot commented Feb 21, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (6086bf2) 15.99% compared to head (801581c) 15.99%.
Report is 6 commits behind head on release/v0.23.6.

Files Patch % Lines
.../core/src/components/Filters/StringFilter/state.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff                @@
##           release/v0.23.6    #4098   +/-   ##
================================================
  Coverage            15.99%   15.99%           
================================================
  Files                  732      732           
  Lines                82062    82062           
  Branches              1110     1110           
================================================
  Hits                 13123    13123           
  Misses               68939    68939           
Flag Coverage Δ
app 15.99% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@brimoor brimoor left a comment

Choose a reason for hiding this comment

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

LGTM (static review based on attached screenshots)

@brimoor brimoor merged commit 528ee14 into release/v0.23.6 Feb 26, 2024
13 checks passed
@brimoor brimoor deleted the bug/slice-change branch February 26, 2024 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Issues related to App features bug Bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants