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

Design: More Space and Visual Hierarchy #4068

Merged
merged 6 commits into from
Oct 31, 2024

Conversation

TimKochDev
Copy link

This pull request is the successor of #3877 which I accidentally closed when trying to rebase onto the next branch. Sorry.

This pull request contains a few small changes that hopefully improve the design of Coolify's UI.
As design is always subjective, I'd understand if you don't like some or all of my suggestions.
Please feel free to decline the pull request or use only fractions of it if that suits you better.

Changes

  • Applies tailwind's shadow to .box class
  • Gives more space to navbar
  • Makes h1 headings larger
  • Add CSS rule to apply margin bottom to <section> elements to semantically and visually group related content
  • Utilize <section>s on Dashboard page

Implementation and Rationals

I tried my very best to keep the number of modified lines as small as possible.
Hence, I kept the original design decision of using a fixed width navbar and only changed this width.

I wish I would have found a single line of code I could change in app.css to introduce more spacing on all pages without editing them (something like every heading element that is not first child gets a top margin).
Unfortunately, I wasn't able to find such a solution because there was no pattern that would always fit.
For example, the profile page features two consecutive form elements with headings inside them (so no headings in the normal page flow).

Since no single CSS selector would always fit, I decided not to clutter the app.css with many such selectors but instead only insert a universally applicable css rule to add bottom margin to <section>.
Then, I edited the dashboard to utilize such sections.

Would you have liked a different approach?

Todo

The pull request can be merged right away.
If you would like me to split the pull request into even smaller ones, please tell me.
I would love to add spacing to other pages as well (like settings and profile) if you agree!


Discussion: #3879
Previous pull request (accidentally closed): #3877

@peaklabs-dev peaklabs-dev self-assigned this Oct 30, 2024
@peaklabs-dev
Copy link
Member

I don't know about you, but the new grey background makes it harder for me to read in dark mode than it was before (especially the grey text) - can we adjust this somehow?

@peaklabs-dev peaklabs-dev added the 💤 Waiting for feedback Issues awaiting a response from the author. label Oct 31, 2024
Makes .box elements appear raised above the background.
Only works in light mode!
"Coolify" had pl-3 while the SwitchTeam element and the menu items below had px-2
Especially in full width and mobile view, the navbar looked cramped
because there was very little padding to the left end of the screen.
This commit adds horizontal padding to the navbar
and increases its width from 48 to 56 tailwind units.
This is to make it even easier to visually distringuish between h1 and other heading elements.
For example, on pages with a h1 heading and multiple section headings,
this makes it easier to understand the structure of the page subconciously.
I think Coolify's design would benefit from a bit more space
between groups of content throughout the site.
HTML's section elements are a perfect fit to
a) semantically group related content and
b) add spacing/visual coherence to it at the same time
@TimKochDev
Copy link
Author

TimKochDev commented Oct 31, 2024

You are absolutely right! Thanks for spotting.

I do indeed think that boxes must become lighter at some point to increase contrast with the background (the boxes are barely visible depending on the screen and lighting). But this was just an experiment and should not have been part of the pull request. Sorry! I removed that change from the pull request.

What do you think about the other changes? Again, feel free to criticize.

@peaklabs-dev
Copy link
Member

No need to apologize - you are fine, and thanks for the PR adjustment. Yes, I also think we need to do some UI and UX fixes in the future, and a complete new UI is planned at some point (opt in only at first). I think it all looks good now. I will merge it. Thanks for the PR.

@peaklabs-dev peaklabs-dev merged commit 231ce49 into coollabsio:next Oct 31, 2024
1 check passed
@github-actions github-actions bot removed the 💤 Waiting for feedback Issues awaiting a response from the author. label Oct 31, 2024
@TimKochDev
Copy link
Author

Yes, I also think we need to do some UI and UX fixes in the future, and a complete new UI is planned at some point (opt in only at first).

Did you note down your plans somewhere already (in a discussing perhaps that I can follow)?
I'd love to keep contributing to such a great project.

"Complete new UI" always sounds risky...

@peaklabs-dev
Copy link
Member

Yes, which is why it will only be opt-in only until we make sure it works properly - offers significant better UX... There is no discussion on GitHub as far as I know, but there is a thread on Discord: https://discord.com/channels/459365938081431553/957753859311300759/1257283946752905246

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.

2 participants