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

Misc refactors to FileSelector #2468

Merged
merged 1 commit into from
Oct 28, 2021
Merged

Misc refactors to FileSelector #2468

merged 1 commit into from
Oct 28, 2021

Conversation

cmdcolin
Copy link
Collaborator

This is a small refactor to fileselector to try to make the code a little more compact

  • Uses a useState for anchorEl instead of useRef, a pattern seen in mui docs pretty often
  • Shorter variable names, just improves code density and reading comprehension a bit (IMO)
  • Removes the hierarchy of Popper-> Grow->Paper->ClickAwayListener->Menu, instead just having Menu which already has effects on it

@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Oct 28, 2021
@codecov
Copy link

codecov bot commented Oct 28, 2021

Codecov Report

Merging #2468 (7087d59) into main (473b37b) will increase coverage by 0.00%.
The diff coverage is 39.39%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2468   +/-   ##
=======================================
  Coverage   61.15%   61.16%           
=======================================
  Files         539      539           
  Lines       25072    25062   -10     
  Branches     5858     5855    -3     
=======================================
- Hits        15332    15328    -4     
+ Misses       9422     9416    -6     
  Partials      318      318           
Impacted Files Coverage Δ
packages/core/ui/FileSelector/FileSelector.tsx 58.46% <39.39%> (+2.46%) ⬆️
products/jbrowse-web/src/util.ts 27.27% <0.00%> (ø)

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 473b37b...7087d59. Read the comment docs.

@cmdcolin cmdcolin removed the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Oct 28, 2021
@cmdcolin
Copy link
Collaborator Author

@peterkxie said the dropdown example comes from https://mui.com/components/button-group/#split-button but it should be fine to remove the wrapping elements

@cmdcolin cmdcolin merged commit 14d1268 into main Oct 28, 2021
@cmdcolin cmdcolin deleted the file_selector_refactors branch October 28, 2021 21:44
cmdcolin added a commit that referenced this pull request Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant