bigbite / themer

GNU General Public License v2.0
2 stars 0 forks source link

Make the sticky feature optional #57

Closed squarewave17 closed 5 months ago

squarewave17 commented 6 months ago

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

g-elwell commented 6 months ago

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 commented 5 months ago

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 commented 5 months ago

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

Might make it more manageable

http://bigbite.im/v/TcidUW

g-elwell commented 5 months ago

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:

https://github.com/bigbite/themer/assets/41474928/ef0d00f2-b08c-48c7-b0c6-d186a943b1fc

thatisrich commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

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

g-elwell commented 5 months ago

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!