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

Make the sticky feature optional #57

Closed
squarewave17 opened this issue May 8, 2024 · 8 comments
Closed

Make the sticky feature optional #57

squarewave17 opened this issue May 8, 2024 · 8 comments
Labels
enhancement New feature or request
Milestone

Comments

@squarewave17
Copy link
Contributor

What problem does this address?

The fix for issue 52 adds an additional problem where preview screens that extend the height of the page will be awkward to view.

I discussed this issue with @jonnywatersbb and we decided that seeing as 99% of the time the preview will be contained within the page, it was better to run with the sticky feature before looking at this fix.

What is your proposed solution?

We should add a button to turn sticky off when needed
Annotation on 2024-05-08 at 15-50-24

@squarewave17 squarewave17 added the enhancement New feature or request label May 8, 2024
@g-elwell
Copy link
Member

g-elwell commented May 8, 2024

I think my preference would be to add overflow: scroll and a max-height to the preview, so if it extends beyond the height of the viewport, you can scroll within it to see the extended content.

Alternatively, we could apply a similar technique to the navigation and style panels, allowing the preview to scroll with the window as per default behaviour, but avoiding the original issue of the style panel causing the page to become long.

Either way I feel like it would be good to avoid adding a toggle for the sticky behaviour if possible

@jonnywatersbb
Copy link
Member

Worth revisiting again in line with #62 as we're now going to have a much longer 'preview' with the full theme.json file

@squarewave17
Copy link
Contributor Author

What if we split the UI by theme.json properties? @jonnywatersbb

Might make it more manageable

http://bigbite.im/v/TcidUW

@g-elwell
Copy link
Member

g-elwell commented Jun 7, 2024

I think my preference would be to add overflow: scroll and a max-height to the preview, so if it extends beyond the height of the viewport, you can scroll within it to see the extended content.

Alternatively, we could apply a similar technique to the navigation and style panels, allowing the preview to scroll with the window as per default behaviour, but avoiding the original issue of the style panel causing the page to become long.

Here's a mockup of what this would look like with a few CSS changes. I think this would be the quickest and safest option for the time being:

Screen.Capture.on.2024-06-06.at.20-28-40.mp4

@jonnywatersbb jonnywatersbb added this to the v1.0.0 milestone Jun 21, 2024
@thatisrich
Copy link
Contributor

Following on from @g-elwell's suggestion, if the panel overflow scrolling is implemented, it might be useful on the UI side to visually retain the level structure within the expanded elements. I've created a rough demo which shows how this could work, also using CSS position sticky, and inherits down the expanded element tree.
Potential issues / gotchas with this approach (as seen in the demo) is the z-index of the parent - child elements causing overlap and also parent element padding causing offset positioning to the sticky elements.

@g-elwell
Copy link
Member

@thatisrich that's a brilliant suggestion, I love it!

I would like to split that into it's own issue if you don't mind creating one? I'd like us to implement it at some point after the v1 release.

@thatisrich
Copy link
Contributor

Great stuff, I'll create a separate issue and refine the approach!

@g-elwell
Copy link
Member

As per my earlier comments, I feel overflow scrolling is a better solution than a sticky toggle.

This issue is therefore superseded by #66 and resolved by #67 so closing it off. Thanks all!

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

No branches or pull requests

4 participants