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

Filters display in Report Timeline view (#5636) #5667

Merged
merged 4 commits into from
Jan 31, 2024
Merged

Conversation

CelineSebe
Copy link
Member

@CelineSebe CelineSebe commented Jan 24, 2024

Proposed changes

  • I suggest adding the ability to expand the bar when the number of filters becomes significant. This would provide a better user experience.

Related issues

Checklist

  • I consider the submitted work as finished
  • I tested the code for its functionality
  • I wrote test cases for the relevant uses case
  • I added/update the relevant documentation (either on github or on notion)
  • Where necessary I refactored code to improve the overall quality

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

Copy link

codecov bot commented Jan 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6ba7cfe) 64.72% compared to head (b4d3416) 64.71%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5667      +/-   ##
==========================================
- Coverage   64.72%   64.71%   -0.01%     
==========================================
  Files         516      516              
  Lines       60893    60893              
  Branches     4857     4855       -2     
==========================================
- Hits        39414    39409       -5     
- Misses      21479    21484       +5     

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

@CelineSebe CelineSebe self-assigned this Jan 26, 2024
@CelineSebe CelineSebe linked an issue Jan 26, 2024 that may be closed by this pull request
@CelineSebe CelineSebe added the filigran team use to identify PR from the Filigran team label Jan 26, 2024
@CelineSebe CelineSebe marked this pull request as ready for review January 26, 2024 14:02
Copy link
Member

@marieflorescontact marieflorescontact left a comment

Choose a reason for hiding this comment

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

I find it a little odd not to display all the filters if you have enough space to do so.
IMO we should choose between tooltip and the ability to expand the bar.
Capture d'écran 2024-01-26 174946

@SarahBocognano
Copy link
Member

I agree with @marieflorescontact, also can you maybe just move the 'close' button because I think it's kind of overlap
Capture d'écran 2024-01-29 105217

@CelineSebe
Copy link
Member Author

CelineSebe commented Jan 29, 2024

It's a quick fix to display all the filters that were hidden before, only on this screen. The filter team is currently changing the display of each filter.

@CelineSebe
Copy link
Member Author

@marieflorescontact @SarahBocognano I reviewed this issue with JP. This problem exists on all screens where we have filters. However, he thinks that the chevron is not necessary. Thank you for your feedback.

@CelineSebe CelineSebe marked this pull request as draft January 30, 2024 08:05
@CelineSebe
Copy link
Member Author

@SarahBocognano @marieflorescontact I have implemented some changes, could you please provide a new review for these updates?

@CelineSebe CelineSebe marked this pull request as ready for review January 31, 2024 15:22
@SamuelHassine SamuelHassine merged commit 451c0d9 into master Jan 31, 2024
8 checks passed
@SamuelHassine SamuelHassine deleted the issue/5636 branch January 31, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
filigran team use to identify PR from the Filigran team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filters display in Report Timeline view
4 participants