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

Debug bar: Dark/light mode + Complete CSS refactoring #2478

Merged
merged 19 commits into from
Feb 23, 2020

Conversation

LittleJ
Copy link
Contributor

@LittleJ LittleJ commented Jan 8, 2020

Description

Many devices and applications have a "dark mode" nowadays. I use it on my computer, and I develop applications with this feature.

When I use the current debug bar, it sometimes gives a contrast which is tiring for the eyes. So I thought it might be interesting to add this feature, which ultimately requires only a few lines of code.

Colors are similar to Chrome's console, using dark mode on Mac OS.

Checklist:

  • Securely signed commits
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide
  • Component(s) with PHPdocs (N/A)

@MGatner
Copy link
Member

MGatner commented Jan 8, 2020

I like the concept, especially as a dark user myself! You might also want to set defaults with a CSS media query, so "unspecified" tries to match the browser/OS setting rather than defaulting to light:

@media (prefers-color-scheme: dark) {

}

@LittleJ
Copy link
Contributor Author

LittleJ commented Jan 8, 2020

Thank you for your feedback ! But suddenly it was a little bit more "complicated" to achieve :-) Indeed...

1 / By default, we use CSS and media queries of course
2 / But we must allow the user to change the mode if he wants to. A "light mode user" may want to use the dark mode, and a "dark mode user" may want to use the light mode.

So here is the result I get.

PS: I decided to apply the theme only to the toolbar, not the icon. Not sure it is the best choice, but even with "dark mode" on, I like when it is white because it is more visible. And that is what I want when I am in a development environnement. A discreet toolbar, but with a clear access. Do you agree ?

@natanfelles
Copy link
Contributor

Just for inspiration: https://github.com/JCSama/CodeIgniter-develbar

@LittleJ
Copy link
Contributor Author

LittleJ commented Jan 8, 2020

https://github.com/JCSama/CodeIgniter-develbar

Thank you, indeed :-) Let's finish this work on dark/light mode, and maybe, if people are interested, we could work on design improvements for the debug bar. I would be glad to ;-)

@MGatner
Copy link
Member

MGatner commented Jan 9, 2020

Yeah this is looking great! I think the white icon is fine, it is very small so not too glaring.

Is this ready to go? More changes you wanted to make?

@MGatner
Copy link
Member

MGatner commented Jan 9, 2020

Actually one request: the font color should be specified for dark mode. If it inherits the color it could end up being unreadable. Example: this actual dark-themed CI4 site using your updates.
Screen Shot 2020-01-09 at 2 35 15 PM

@LittleJ
Copy link
Contributor Author

LittleJ commented Jan 10, 2020

Thank you for your feedback ! Here is the thing...

I wanted to add the "dark/light" feature, without changing colors. Because, when I worked on the css file, there are many things that came through my mind. There is really place for improvement. And there is @natanfelles 's comment too, talking about future design improvements that could be done.

So, I started to write things down (brainstorming). And I would really appreciate if you could take a few minutes to have a look at this: https://littlej.github.io/ci4-debug-bar-design/build/html/

Then, you tell me what you think :-) I am OK adding colors, but maybe it would be better to agree on the "color scheme" first ?

Thank you again for the time you are giving me !

@LittleJ
Copy link
Contributor Author

LittleJ commented Jan 10, 2020

Please note that there are some similar issues right now with the current css.

Simple example:
1/ Load CI default page (fresh install)
2/ Add a style to body with a color, like this: body { color: red; }

And now you get a toolbar with red labels :-D

That is one of the reasons I started doing those suggestions. Because I create apps with CSS. It is all about user experience. And when I am done doing this, I open the console and sometimes find a weird interface :-D And I don't like to have Debugbar styles in the app's css, that's what is happening today :-)

Maybe the "dark/light" feature is ready. But the work is more on deep css consolidation (cf my previous post).

@lonnieezell
Copy link
Member

I like this idea a lot. And agree there are plenty of places for improvement in the Toolbar.

Are you all happy with the place this is at currently or should we still expect more changes to come soon?

@LittleJ
Copy link
Contributor Author

LittleJ commented Jan 13, 2020

Note: I am willing to work on CSS improvements. Do not hesitate to tell me if you want me to continue.

@lonnieezell
Copy link
Member

@LittleJ yes, please continue. :)

@LittleJ
Copy link
Contributor Author

LittleJ commented Jan 16, 2020

@MGatner @lonnieezell Hey ! Sorry for the late answer, I was on a business trip in Paris. I did continue, and I am still working on the new stylesheet, as I am honored to contribute to CI. You can count on me, but I just need a few more days, because I did not have much spare time this week. Thank you for your comprehension ! ;-)

@lonnieezell
Copy link
Member

No worries - we have a little bit of time. I'm shooting to get RC-4 out by end of next week, assuming that we get the release figured out in time.

I'm going to change the title to WIP. When you've got it ready, remove that and let me know and I'll take a gander.

@lonnieezell lonnieezell changed the title Dark mode for the debug bar WIP - Dark mode for the debug bar Jan 17, 2020
@LittleJ
Copy link
Contributor Author

LittleJ commented Jan 19, 2020

Hello there !

It took me hours to do the complete refactoring of the CSS. I tried to stay as close as possible to the current design, while making a significant number of improvements.

Here are the main new features:

  • Conversion of CSS into SASS version
  • Added many detailed comments
  • Added variables (example: you can change "$ base-size: 16px;" and this automatically modifies the other elements: titles, paragraphs, tables' content, etc...)
  • Color cleaning and standardization (with the definition of the graphic charter)
  • Standardization of measurements (there were mixes between "em" and "px")
  • Standardization of views between the different tabs (spacing, rendering of tables, colors used ...)
  • Cleaned unused and / or duplicated styles
  • Cross-browser optimizations
  • Preventive measures to prevent conflicts with other stylesheets
  • And countless small improvements ... (examples to illustrate: the toggle icon is now the same size as the toolbar because there was a gap because of the borders, the "responsive breakpoint" for the icons was not adapted to the content of the toolbar ...)

I couldn't do a commit for every change, as it is a complete refactoring, in another "language" (SASS).

I really hope you will like it ! :-)

@LittleJ LittleJ changed the title WIP - Dark mode for the debug bar DEBUG BAR - Dark mode + Complete CSS refactoring Jan 19, 2020
@LittleJ
Copy link
Contributor Author

LittleJ commented Feb 3, 2020

@lonnieezell With an uncompressed version of the CSS, there is no problem for the developers. It is the same thing as today. But, what to do with these SASS files (used by core developers/contributors) ? There are 4 solutions I see:

1/ Use Github to generate the CSS version (one of your initial ideas). Which i found not totally adapted as the developers have to see the result before pulling it. So it is not really a "complete" solution.

2/ Ask them to generate the CSS from the SASS version. You have rules for PHP contributions, there could be one for CSS contributions too.

3/ Remove the SASS version. Maybe put it in a separated repo. But the issue will only be moved elsewhere.

4/ Forget about the SASS version. We only keep the final CSS. I just lose part of my work. It happens... too bad for me :-)

Note that if we create a new homepage (other thread), we will have the same CSS issues. The homepage should use the same graphic chart that has been defined... The homepage CSS could even be the same as the debug bar CSS. Meaning there is only one big CSS file for the whole framework, which will just be included in the homepage too (it is possible).

If you don't want complications, the simplest solution is just to keep the CSS version only. But it will probably leave less space and ease for future improvements.
If you want a simple solution, while keeping the SASS version, you make it mandatory for core developers/contributors to use it. And you can still change your mind later and remove the SASS version.
Else, there are solutions, but none is "simple"... :-)

@LittleJ
Copy link
Contributor Author

LittleJ commented Feb 13, 2020

@lonnieezell I was thinking about something like this too, for the settings. What do you think ?

Capture d’écran 2020-02-13 à 19

@lonnieezell
Copy link
Member

lonnieezell commented Feb 13, 2020 via email

@LittleJ LittleJ changed the title DEBUG BAR - Dark mode + Complete CSS refactoring WIP - Debug bar: Dark/light mode + New menu for the settings + Complete CSS refactoring Feb 13, 2020
@MGatner
Copy link
Member

MGatner commented Feb 17, 2020

@LittleJ I forked your branch and tried it on the new CI4 Welcome Page. It looks really good! On question on behavior... it seems to be dark by default and regardless of my OS setting, unless I change it with the toolbar-theme button. Is that intended? I had suggested the prefers-color-scheme: dark media query above - its fine if you decided that was too much but I want to make sure that if that is the intended behavior it isn't a bug.

Re: SASS I definitely think hanging onto the SASS files in the main repo is a boon! What do you and @lonnieezell think about this: move the instructions for generating the CSS files into their own file in the contributing folder and add the SASS files there in a sub-folder as well.

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

Changes mentioned in my comment above

@lonnieezell
Copy link
Member

@MGatner I like the idea of moving it into a subfolder. We do have to ensure we update those as part of the release cycle, still, though. Handled through the scripts in /admin. That can always be a different PR, though, as I'd love to see this included before the hopeful release on the 24th.

@LittleJ
Copy link
Contributor Author

LittleJ commented Feb 19, 2020

Hey ! Sorry, I was on a business trip...
I think it is going to be complicated to do all the modifications (dropdown menu) before the 24th. And maybe some other members will want to verify the code and pass into some sort of validator... It could take time. Maybe we could stick to this version for now (dark mode + CSS refactoring), and improve it after (dropdown menu + fullscreen mode).
You decide what is best, and I will do my best to help out :-)

PS: I find it complicated to see the changes you made on Github. So I mainly rely on your comments.

@lonnieezell
Copy link
Member

If we can get just the CSS + dark mode refactor that would be awesome. And if that could be reorganized like @MGatner mentioned that would be awesomer. :)

@LittleJ
Copy link
Contributor Author

LittleJ commented Feb 19, 2020

I am a little confused... Do I have to make the changes based on the fork ? Or do I have to confirm a pull request somewhere ? What is the next step ? :-)

@LittleJ
Copy link
Contributor Author

LittleJ commented Feb 19, 2020

By the way, thank you for your answers on the other pull request @lonnieezell. I was clearly not using the right words to express myself. I got confused too, my english is sometimes limited :-)

@lonnieezell
Copy link
Member

Nope - no one has submitted PRs against your PR. I believe the only changes have been in comments.

Basically we are just looking for a PR with what you have plus any changes just recently asked for.

@MGatner
Copy link
Member

MGatner commented Feb 21, 2020

@LittleJ We would love to have this in the initial release! Could you (before Monday) (before Sunday night actually) realistically get these tasks done?

  1. Change the light/dark mode default to be dependent on @media prefers-color-scheme: dark
  2. Create a new file, contributing/css.rst
  3. Move instructions from system/Debug/Toolbar/Views/SASS/toolbar.scss to the new doc and add anything else someone might need to know
  4. Create a new folder, admin/css/ and add the SASS files (and anything else needed to generate the CSS)

@LittleJ
Copy link
Contributor Author

LittleJ commented Feb 22, 2020

Yes I think it is possible. I just had to wait for the week-end to have a little bit more time ;-)

@LittleJ
Copy link
Contributor Author

LittleJ commented Feb 22, 2020

I will send the changes later today. And I will be available for fixes/revisions if needed (including tomorrow).

@MGatner
Copy link
Member

MGatner commented Feb 22, 2020

That’s great news! Thanks so much - I’m excited this will be part of the launch.

@LittleJ
Copy link
Contributor Author

LittleJ commented Feb 22, 2020

@MGatner I cannot reproduce your bug on my mac. If i clear the cache, I get the right color by default (light), the one that is set in the system's settings. Is it possible that you used the "switch theme" button and the cookie was still alive ? Because the color is stored like the position.

@LittleJ LittleJ requested a review from MGatner February 22, 2020 16:16
@LittleJ
Copy link
Contributor Author

LittleJ commented Feb 22, 2020

@lonnieezell While we wait for an answer from @MGatner, I moved the instructions. Could you have a look and check if my translations are good ? Thank you !

@LittleJ LittleJ changed the title WIP - Debug bar: Dark/light mode + New menu for the settings + Complete CSS refactoring Debug bar: Dark/light mode + Complete CSS refactoring Feb 22, 2020
@MGatner
Copy link
Member

MGatner commented Feb 22, 2020

Okay that's great! No worries about my experience, I could have switched theme earlier like you said. I'm a CSS lightweight so I trust you to know what you're doing.

The docs and SCSS files look good! I'll let Lonnie have one last look at it but we'll plan to merge it tonight barring any further input. Thank you so much, this is a great asset to the framework

@LittleJ
Copy link
Contributor Author

LittleJ commented Feb 22, 2020

No, no, it is my pleasure ! :-) I will do more improvements after this release. It is just a question of time/deadline.

@lonnieezell
Copy link
Member

Looks great! Thanks so much. Merging.

@lonnieezell lonnieezell merged commit 20b3d78 into codeigniter4:develop Feb 23, 2020
totoprayogo1916 added a commit to totoprayogo1916/framework-CI4 that referenced this pull request Nov 10, 2020
paulbalandan pushed a commit that referenced this pull request Nov 16, 2020
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.

4 participants