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

Add collapsible accordion sections in configuration editor #2796

Merged
merged 9 commits into from
Mar 15, 2022

Conversation

cmdcolin
Copy link
Collaborator

This is a possible helper for #1402

Adds collapsible sections to the config editor. All sections are expanded by default, but they can be collapsed. The animation on collapsing is disabled so it happens quickly.

I wasn't able to make the configuration editor for the long callback strings get smaller (be contained in the width of the drawer) but you can sidescroll now (overflow:none is removed)

live link
https://jbrowse.org/code/jb2/accordion_config_editor/?config=test_data%2Fvolvox%2Fconfig.json&session=share-BfuDrTSD5P&password=LFGyt

@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Mar 11, 2022
@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 Mar 11, 2022
@codecov
Copy link

codecov bot commented Mar 11, 2022

Codecov Report

Merging #2796 (8f93aca) into main (b183df6) will decrease coverage by 0.00%.
The diff coverage is 86.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2796      +/-   ##
==========================================
- Coverage   59.98%   59.97%   -0.01%     
==========================================
  Files         584      584              
  Lines       26676    26679       +3     
  Branches     6454     6458       +4     
==========================================
  Hits        16002    16002              
- Misses      10345    10348       +3     
  Partials      329      329              
Impacted Files Coverage Δ
...kages/core/BaseFeatureWidget/BaseFeatureDetail.tsx 76.15% <ø> (ø)
packages/core/ui/theme.ts 100.00% <ø> (ø)
...ConfigurationEditorWidget/components/SlotEditor.js 63.15% <ø> (ø)
...igurationEditorWidget/components/CallbackEditor.js 70.00% <50.00%> (+2.25%) ⬆️
...tionEditorWidget/components/ConfigurationEditor.js 87.50% <92.30%> (-1.79%) ⬇️
...s/svg/src/SvgFeatureRenderer/components/Chevron.js 87.17% <0.00%> (-10.26%) ⬇️
packages/core/util/analytics.ts 89.79% <0.00%> (-2.05%) ⬇️
...gins/svg/src/SvgFeatureRenderer/components/util.ts 92.72% <0.00%> (-1.82%) ⬇️
...gFeatureRenderer/components/ProcessedTranscript.js 85.13% <0.00%> (-1.36%) ⬇️
packages/core/util/layouts/GranularRectLayout.ts 86.75% <0.00%> (-0.43%) ⬇️
... and 3 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 b183df6...8f93aca. Read the comment docs.

@cmdcolin
Copy link
Collaborator Author

I considered whether it is desirable to limit the width of the callback editor to the width of the parent, and what the behavior might be. It seems like the two things that might be ideal is that it would wrap, but jexls are single line, or it extends to the right. If it extends to the right, that means that individual element could have a side scroll, or the entire side panel side scrolls. This PR has the entire side panel side scroll. This helps avoid a bit of "too many scroll bars" ...just a single side scrollbar for the drawer.

@cmdcolin
Copy link
Collaborator Author

summary: this PR, which does not limit the width of the callback editor, could perhaps be better than trying to limit the width of the callback editor

@cmdcolin
Copy link
Collaborator Author

this branch
Screenshot from 2022-03-10 21-53-53

main
Screenshot from 2022-03-10 21-54-01

@cmdcolin
Copy link
Collaborator Author

curious what @garrettjstevens and @scottcain might think, follows from discussion yesterday

@garrettjstevens
Copy link
Collaborator

I think collapsible sections could be nice, but with this PR I can't really see the "hierarchy" of the config sections (e.g. that "textSearchAdapter" is nested under "textSearching"). I use that hierarchy a lot to navigate the config editor, so it would be nice for me to be able to see that better.

@cmdcolin
Copy link
Collaborator Author

In the old hierarchical track selector I think we specifically put a darker color on the accordion colors...could probably do that here too

@cmdcolin
Copy link
Collaborator Author

Has darker borders now

before
Screenshot from 2022-03-14 16-25-42

after
Screenshot from 2022-03-14 16-25-22

@garrettjstevens
Copy link
Collaborator

That border helps me, although might I suggest border: `1px solid ${theme.palette.text.primary} ` for the border? That way it should have good contrast in dark mode as well.

@cmdcolin
Copy link
Collaborator Author

Added the "config path"

example with adapter, adapter->index
Screenshot from 2022-03-14 17-27-34

@garrettjstevens
Copy link
Collaborator

I like the config path, it makes it easier to understand what I'm looking at.

What would you think if we enabled the collapse/expand animation, but made it shorter? The default looks like it's 300, and I think as short as 150 it looks fine. I've noticed that the animation helps me to navigate more easily.

@cmdcolin
Copy link
Collaborator Author

150ms is good, feels fairly fast. pushed to here

@cmdcolin
Copy link
Collaborator Author

maybe can merge! great feedback @garrettjstevens

Copy link
Collaborator

@garrettjstevens garrettjstevens left a comment

Choose a reason for hiding this comment

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

👍 from me

@cmdcolin cmdcolin merged commit 587c634 into main Mar 15, 2022
@cmdcolin cmdcolin changed the title Collapsible accordion sections in configuration editor Add collapsible accordion sections in configuration editor Mar 15, 2022
@cmdcolin cmdcolin deleted the accordion_config_editor branch March 15, 2022 16:30
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.

2 participants