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 section collapse functionality #1772

Merged
merged 11 commits into from Mar 13, 2023
Merged

Conversation

jannikbecher
Copy link
Contributor

WIP #1686

screen_recording_livebook_collapse_section.webm

Is this heading in the right direction?
Not sure about the "Expand all" and "Collapse all" buttons..

@github-actions
Copy link

github-actions bot commented Mar 11, 2023

Uffizzi Preview deployment-18734 was deleted.

@josevalim
Copy link
Contributor

@jannikbecher this looks like a great starting point. We can tackle the collapse and expand all in another pull request. Two things:

  1. UI wise, the starting of each section needs to be aligned with the title. So we want to have the collapse button "outside". Something like this:

image

  1. We can add a note with how many cells were hidden

image

@jonatanklosko do you think this should be fully handled client side? I guess we can add a class when collapsing and then remove it. We change the content accordingly based on the presence of said class?

@jannikbecher
Copy link
Contributor Author

jannikbecher commented Mar 11, 2023

@josevalim sorry I'm complete beginner in UI and tailwind...
should I handle the edge case of "1 cell hidden"?

does this look ok?
Screenshot_20230311_163009

@jonatanklosko
Copy link
Member

@jonatanklosko do you think this should be fully handled client side?

Yeah, one option would be adding a class to section and some conditional rules (as we do in js_interop.css). Should also be doable with JS commands and I don't have a strong preference, but probably less brittle with conditional CSS than updating individual elements.

@jannikbecher
Copy link
Contributor Author

Ahh I see. I completely misused LiveView functionality here. I will come up with a solution the next days using js_interop.css as @jonatanklosko described :)

@jannikbecher
Copy link
Contributor Author

Screenshot_20230312_174424

Unfortunately I'm really struggling with the alignment. Is it even possible to align it probably when the expand/collapse buttons are defined in section_component.ex?
I removed the collapse/expand all functionality for now. Any preference on how the buttons should look like?

@jannikbecher
Copy link
Contributor Author

Screenshot_20230313_085810

I made some progress regarding the alignment :)

@jonatanklosko
Copy link
Member

@jannikbecher awesome! I moved the button closer to the title and made it hidden unless collapsed or hovered :)

collapse_section.mov

@jannikbecher
Copy link
Contributor Author

This looks so much nicer! Thanks for the patience. I learned so much the last couple of days and it was such a joy :)

Copy link
Contributor

@josevalim josevalim left a comment

Choose a reason for hiding this comment

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

Two minor suggestions for @jonatanklosko to consider and ship it!

Thanks @jannikbecher ❤️

jonatanklosko and others added 2 commits March 13, 2023 14:46
Co-authored-by: José Valim <jose.valim@gmail.com>
@jonatanklosko jonatanklosko merged commit cea3246 into livebook-dev:main Mar 13, 2023
6 checks passed
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

3 participants