glanceapp / glance

A self-hosted dashboard that puts all your feeds in one place
GNU Affero General Public License v3.0
8.56k stars 297 forks source link

CSS dynamic columns refactor and made more widgets compatible with them #178

Closed DVDAndroid closed 2 months ago

DVDAndroid commented 3 months ago

I cleaned up some CSS code related to dynamic columns classes and made Github releases, Changedetection, monitor, Twitch channels and categories widgets compatible with dynamic-columns-experimental

Some previews:

One column sample ![1c](https://github.com/user-attachments/assets/6d1eb505-6eaa-4f73-bc37-2e793d071fcf)
5 columns sample ![5c](https://github.com/user-attachments/assets/eb7c6639-a3c7-4f47-9f80-45cd2974c3f8)
svilenmarkov commented 3 months ago

Thanks for contributing!

Refactor might not be the right word since this changes the previous intended behavior. Admittedly, the way dynamic columns should behave is somewhat subjective.

I wanted elements to be equally distributed horizontally regardless of whether you had 2 or 5 elements (any more than that and they go on a new row (depending on available width)). CSS isn't really good at doing different things depending on the number of children which is why the implementation looks a bit yuck. The end result however is in my opinion a mostly pleasant and balanced usage of space:

1

Compared to the left leaning implementation here:

2

Obviously this is just one example, there are scenarios where the layout is the same and others where your implementation looks better. Both have ups and downs. I really like your usage of the border to separate the elements which is something I wanted to do but couldn't figure out a sane way of doing that didn't require a ton of boilerplate due to the variable number of children, so I had to settle with a less than ideal background color to indicate separation. In some scenarios your implementation also does a better job at retaining the aesthetic of the widgets by limiting the elements' width which matches how they would normally look in a small column, compared to the horizontally elongated look they have currently.

I don't know what to do honestly. Should the user have the option to choose between the two?

DVDAndroid commented 3 months ago

Refactor might not be the right word since this changes the previous intended behavior.

I wanted elements to be equally distributed horizontally regardless of whether you had 2 or 5 elements (any more than that and they go on a new row (depending on available width)). CSS isn't really good at doing different things depending on the number of children which is why the implementation looks a bit yuck. The end result however is in my opinion a mostly pleasant and balanced usage of space:

1

To be honest, I never noticed the "space-between" behavior maybe because I used the monitor widget with 6+ services. However, I just pushed a commit that restores the original spacing behavior.

I don't know what to do honestly. Should the user have the option to choose between the two?

In my opinion, yes. Somebody may like it compact, others may like it spaced.

svilenmarkov commented 3 months ago

I made a bunch of changes but I'll withhold from merging since there's a bunch more stuff I want to sort out:

DVDAndroid commented 2 months ago

With the new release 0.6.0 I can close this PR. Also I noticed that: some padding has been changed so homepage appears a little more compact releases widget doesn't adapt horizontally in "full" mode

svilenmarkov commented 2 months ago

You are correct that the releases widget doesn't have the dynamic columns behavior applied to it. It's a little more tricky doing it correctly for the widgets which have collapsing behavior and that's something I will address in a future release, but for the time being I didn't want to delay this release any further.