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

Adding Dark Theme #1436

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

Adding Dark Theme #1436

wants to merge 3 commits into from

Conversation

AhmedEid3
Copy link

Adding Dark Theme ref issue #1424

@lubber-de lubber-de added lang/css Anything involving CSS state/awaiting-reviews Pull requests which are waiting for reviews type/feat Any feature requests or improvements labels Apr 25, 2020
@exoego
Copy link
Contributor

exoego commented Apr 28, 2020

Great work !
I suggest to name this theme as dark-default instead of just dark, so users know the theme is dark version of default theme.
And, in future, we may add new dark themes like dark-duo or dark-material.

@AhmedEid3
Copy link
Author

Great work !
I suggest to name this theme as dark-default instead of just dark, so users know the theme is dark version of default theme.
And, in future, we may add new dark themes like dark-duo or dark-material.

its good to rename it to dark-default

Copy link
Member

@lubber-de lubber-de left a comment

Choose a reason for hiding this comment

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

Thank you very much for this PR!

This is a very good start, we really appreciate the effort you spent into this already! 👍
I have prepared a test jsfiddle which covers nearly every component of FUI to be able to test your theme:
https://jsfiddle.net/hpetaxy0/1/show

Same page using the original default FUI Theme (for comparison)
https://jsfiddle.net/hpetaxy0/show

As you will recognize by scrolling through the page there are still some parts left which need cosmetic adjustments:

General

  • I will mention this below a few times: All inverted variants are still unchanged. If we have a dark default theme, i would assume all inverted variants of each component should look the opposite as the default then. IMHO any inverted variant should have its background grey or even white. Something we might need to discuss with the community.

Menu

  • The inverted menu has a teal color as background. Perhaps its only my opinion, but a "inverted" variant of a dark version should be grey or even white, but should not have any specific color (thats why we have all color variants for)

  • An active item text of an inverted menu (regardless of the above mentiond color) is not readable
    image

  • A pointing menu pointer arrow is shown as filled white square
    image

  • Hovering over an active menu item makes the menu entry unreadable because its text switches to black
    image

  • Hovering over a tabular menu item makes the text unreadable, because it switches to black
    image

Dropdown

  • Opening a Dropdown from a menu makes the dropdown totally black making it unreadable
    image

Table

  • Hovering over a selectable table makes the hovering row barely readable because text will be set to black
    image

  • The first column of a definition table still has black text making it unreadable
    image

Card

  • A card header is still black
  • The text/icons in cards extra content is still black
    image

Calendar

  • The calendars/popups pointing arrow is shown as a filled square
    image

  • Disabled calendar days from the previous/next month are not recognizable (same color as usual month days)
    image

Slider

  • The remaining slider line is only barely visible
    image

Modal

  • A modal header is still black
    image

  • An inverted modal is still looking like the default theme (black) and the edges are barely visible.
    image

Toast

  • A default toast text is white, but default background is also still white making it unreadble
  • The default shadow should be lighter when its a dark background , it's still the same as for the default theme
    image

Breadcrumb

  • The default color of links is a bit too dark to me making it barely readable (applies for links in other components as well)
  • Inverted variant still the same
    image

Accordion

  • Link color in opened header too dark (usual link complaint, same as for breadcrump, see above)
  • inverted variant completely untouched (see my above comment for modal)
    image

Feed

  • Same as for Accordion (Links and inverted variant)
    image

Comment

  • Same as above for inverted variant
    image

Tab

  • Hovering over (non active) tabular menu item switches its color to black making it unreadable
    image

@lubber-de
Copy link
Member

@AhmedEid3 Will you continue to work on this PR?

@lubber-de lubber-de added the state/awaiting-response Issues or pull requests waiting for a response label May 15, 2020
@lubber-de lubber-de requested a review from ko2in June 9, 2020 10:06
@lubber-de
Copy link
Member

@AhmedEid3 Again the question, since last time i asked, was 6 weeks ago: Are you interested in finishing this PR and adjusting the requested changes?

@lubber-de lubber-de added the state/on-hold Issues and pull requests which are on hold for any reason label Jul 3, 2020
@lubber-de lubber-de removed the state/awaiting-reviews Pull requests which are waiting for reviews label Jul 23, 2020
@geochronology
Copy link

Hey do you know if this is still actively being worked on? I would be more than happy to collaborate on the remaining changes if it would help get this PR over the finish line.

@lubber-de
Copy link
Member

@geochronology I asked several times as seen above but sadly did not get any response anymore. You are of course free to take the changes into your own PR if you like. 😉

@geochronology
Copy link

@lubber-de Yeah this seems too important of a feature to leave unfinished and I'd be totally down with picking up where the project left off.

Quick question tho: since the PR is almost half a year old by now, are there new commits to the main branch that I should be aware of?

Perhaps we could sync up on Discord to discuss more about it if you like. (I'm Adrien#2343).

@lubber-de
Copy link
Member

Yes, there have been several new commits merged already, which could affect the theming as there probably are new LESS variables since then.

Just make sure your PR is always based on the latest develop branch.

Btw, I could not find your Nick as a member on our discord server. Please join there, if you like. https://discord.gg/YChxjJ3

@arminpkathrein
Copy link

@geochronology I would love to use this in my current project. I've been looking for a light/dark option, but so far i've only been using the "inverted" option. It's great, but it's too much of an contrast from white to black....

@geochronology
Copy link

Thank you to both of you for your replies.

@lubber-de Ah I wasn't aware of the Discord, I'll go ahead and join

@arminpkathrein Thanks for the encouragement ^-^ I'm finishing up a push at work this week. Working on the theme is next up on my bucket list. Realistically I'll probably have this done by the end of the month.

@geochronology
Copy link

Hey just wanted to give an update on this with a bit of context and perspective since I took on this project:

  1. Contract work, RL, and interviews have all been grinding me hard lately which has minimized the time I've been able to spend on incredibly worthy projects like this one. I'm finally coming up for a bit of air.

  2. The theme needs more work than I realized. It is very much a work in progress, and beyond the comments mentioned above would really benefit from a bit of color theory and best practices, not to mention community input.

  3. Getting into this project has made me curious about how theming works under the hood and for my own curiosity, I'm taking the opportunity to understand what Gulp and Less are really doing behind the scenes.

I get not letting the perfect be the enemy of the good, but for something like a dark theme, if I'm putting my name on it I want it to be beautiful, so I'm going to take the time to get it right (while also posting more regular and incremental updates like this one).

Hope this is helpful and I'll do my best to check in by the end of the year with -- if not a PR -- then at least a meaningful status update with some real work to show.

@lubber-de lubber-de marked this pull request as draft May 16, 2021 13:31
@lifeart
Copy link

lifeart commented Feb 9, 2022

Hello! Any updates on this initiative?

@grandeljay grandeljay mentioned this pull request Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/css Anything involving CSS state/awaiting-response Issues or pull requests waiting for a response state/on-hold Issues and pull requests which are on hold for any reason type/feat Any feature requests or improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants