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

Adds "Favorites" and "Recently used" track categories to the track selector #4039

Merged
merged 17 commits into from Nov 22, 2023

Conversation

carolinebridge
Copy link
Contributor

Resolves #3834

Adds categories for "Favorite Tracks" and "Recently Used" to the track selector.

@carolinebridge carolinebridge added the enhancement New feature or request label Nov 3, 2023
@carolinebridge carolinebridge self-assigned this Nov 3, 2023
}),
}))
// always keep the Tracks entry at idx 0
.filter((f, idx) => idx === 0 || !!f.children.length),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this still necessary? I removed it because it was interfering with the ability to toggle Favorites and Recently Used on/off.

This filter is why you couldn't see Recently Used when it was empty

Copy link
Collaborator

Choose a reason for hiding this comment

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

can try removing it if you want. note that the comment suggests that it is used to help "keep Tracks entry at idx 0". that assumption i suppose might be broken if recently used and favorites are "above Tracks"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What was the original motivation behind forcing it to be 0

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it was just a way to force "Tracks" to display even if it was "empty" but the git blame is not particularly helpful, I didn't make an accurate commit message for this change. I would say can probably delete the idx === 0 part, and then deleting the filter entirely could be ok also if we want to allow showing empty categories (including e.g. completely empty connections)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's OK to show empty categories. and important to if we're going to add "functional" categories like these, since they can toggle on and off, and if they're empty and not shown they would be undiscoverable

@carolinebridge
Copy link
Contributor Author

Do we want these categories accessible through the Faceted Track selector?

@cmdcolin
Copy link
Collaborator

cmdcolin commented Nov 3, 2023

for faceted: probably not required but could be a "nice to have". jbrowse 1 faceted selector did have a recently used concept

@carolinebridge
Copy link
Contributor Author

From meeting:

  • option bulk clear favorites and recently used
  • store in localStorage

@carolinebridge carolinebridge marked this pull request as ready for review November 15, 2023 16:02
@carolinebridge carolinebridge changed the title WIP: Adds "Favorite Tracks" and "Recently Used" categories to the track selector Adds "Favorite Tracks" and "Recently Used" categories to the track selector Nov 15, 2023
Copy link

codecov bot commented Nov 15, 2023

Codecov Report

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

Comparison is base (3b6ecff) 0.00% compared to head (621a95f) 63.19%.
Report is 4 commits behind head on main.

Files Patch % Lines
...ement/src/HierarchicalTrackSelectorWidget/model.ts 60.00% 30 Missing and 2 partials ⚠️
...TrackSelectorWidget/components/tree/TrackLabel.tsx 55.55% 3 Missing and 1 partial ⚠️
...ckSelectorWidget/components/tree/HamburgerMenu.tsx 0.00% 2 Missing ⚠️
...ckSelectorWidget/components/tree/TrackCategory.tsx 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           main    #4039       +/-   ##
=========================================
+ Coverage      0   63.19%   +63.19%     
=========================================
  Files         0     1044     +1044     
  Lines         0    30637    +30637     
  Branches      0     7321     +7321     
=========================================
+ Hits          0    19360    +19360     
- Misses        0    11103    +11103     
- Partials      0      174      +174     

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

/**
* #property
*/
showRecentlyUsedCategory: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks like these settings would be shared via share links. it might be worth converting to a localstorage preference

Copy link
Contributor Author

Choose a reason for hiding this comment

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

currently have them separate localstorage items, let me know if you want me to combine them into a single one

@cmdcolin
Copy link
Collaborator

this is a misc UI thing.

do you think recently used and maybe favorites too should be collapsed by default? it can be a slightly jarring to check a box off in the track selector and then it updates the recently used, and by collapsing recently used by default, this experience will be at least deferred to someone who is explicitly interested in looking at the recently used (they expand that category)

@cmdcolin cmdcolin merged commit 1cf6631 into main Nov 22, 2023
11 checks passed
@cmdcolin cmdcolin deleted the favourite-tracks branch November 22, 2023 04:29
@cmdcolin
Copy link
Collaborator

🥳

@cmdcolin
Copy link
Collaborator

merged

@carolinebridge
Copy link
Contributor Author

re: final ui comment -- I thought about this, but I didn't want to limit discoverability. I figured the toggles fulfilled this (user finds recently used is jarring -> turn it off -> basically never have to look at it again)

@cmdcolin
Copy link
Collaborator

there was actually code in there that said isOpenByDefault:false for the recently used/favorites but it was non-working, since the actual source of truth for this is the "collapsed" state, so i removed it. could consider re-making it collapsed by default but will just leave as is

@cmdcolin cmdcolin changed the title Adds "Favorite Tracks" and "Recently Used" categories to the track selector Adds "Favorites" and "Recently used" track categories to the track selector Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a 'favorites' option to the track selector
2 participants