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

Import bookmarks functionality for grid bookmark widget #2243

Merged
merged 4 commits into from
Aug 25, 2021

Conversation

cmdcolin
Copy link
Collaborator

@cmdcolin cmdcolin commented Aug 21, 2021

I had a quick interest in getting a import bookmarks function implemented

This let's a user quickly import a BED formatted file. The name column 4 is stored in the "label" field

It also has a couple more add-ons

  • replaces the NA default export with a . which is the standard not available data description for BED
  • makes it so duplicates in bookmarks are allowed, as this may come from data files, and rather than filtering on import, it retains them (note that if this was not done, then strange errors would occur on editing fields)
  • makes the hover style on links in the grid bookmark widget cursor:pointer

Some motivation to this includes my feeling that you can easily loose track of your session

During development especially, I cycle through tons of sessions and I imagine a user may also, and if we're not careful, we would loose our bookmarks (since bookmarks are currently tied to session...). So if we can at least re-import them, it seems useful

Just an idea I picked up and ran with :)

@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Aug 21, 2021
@cmdcolin cmdcolin added enhancement New feature or request and removed needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) labels Aug 21, 2021
@cmdcolin cmdcolin marked this pull request as draft August 21, 2021 15:08
@cmdcolin cmdcolin marked this pull request as ready for review August 21, 2021 15:43
@codecov
Copy link

codecov bot commented Aug 21, 2021

Codecov Report

Merging #2243 (6ff4aac) into main (60b4ef3) will decrease coverage by 0.04%.
The diff coverage is 58.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2243      +/-   ##
==========================================
- Coverage   62.88%   62.84%   -0.05%     
==========================================
  Files         493      494       +1     
  Lines       22907    22942      +35     
  Branches     5178     5191      +13     
==========================================
+ Hits        14405    14417      +12     
- Misses       8236     8258      +22     
- Partials      266      267       +1     
Impacted Files Coverage Δ
.../GridBookmarkWidget/components/ImportBookmarks.tsx 32.43% <32.43%> (ø)
...GridBookmarkWidget/components/AssemblySelector.tsx 85.71% <50.00%> (-1.79%) ⬇️
...c/GridBookmarkWidget/components/ClearBookmarks.tsx 71.42% <60.00%> (-11.91%) ⬇️
...gins/grid-bookmark/src/GridBookmarkWidget/model.ts 72.41% <66.66%> (-4.06%) ⬇️
...c/GridBookmarkWidget/components/DeleteBookmark.tsx 85.71% <77.77%> (-14.29%) ⬇️
...ridBookmarkWidget/components/DownloadBookmarks.tsx 80.00% <80.00%> (-6.67%) ⬇️
...idBookmarkWidget/components/GridBookmarkWidget.tsx 90.24% <88.23%> (+6.52%) ⬆️
...gins/grid-bookmark/src/GridBookmarkWidget/utils.ts 61.22% <100.00%> (ø)
packages/core/util/io/rangeFetcher.ts 90.24% <0.00%> (-2.44%) ⬇️
packages/core/assemblyManager/assemblyManager.ts 67.10% <0.00%> (-1.32%) ⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 60b4ef3...6ff4aac. Read the comment docs.

@cmdcolin cmdcolin force-pushed the import_bookmarks branch 2 times, most recently from d9924e5 to f8244d8 Compare August 23, 2021 19:46
@elliothershberg
Copy link
Member

Nice! I'll take a look at this soon and give it a spin locally

Copy link
Member

@elliothershberg elliothershberg left a comment

Choose a reason for hiding this comment

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

Overall looks good! I added a few points about the import form, but other than that I think this is a good addition to bookmarks.

@cmdcolin cmdcolin merged commit 61c6041 into main Aug 25, 2021
@cmdcolin cmdcolin deleted the import_bookmarks branch August 25, 2021 01:13
@cmdcolin
Copy link
Collaborator Author

thanks for the review @elliothershberg :) big 🥳 for the bookmark feature as a whole

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.

None yet

2 participants