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

Needs light mode #9

Open
sreekaransrinath opened this issue Oct 14, 2022 · 23 comments
Open

Needs light mode #9

sreekaransrinath opened this issue Oct 14, 2022 · 23 comments

Comments

@sreekaransrinath
Copy link
Contributor

Dark background + light text is not very accessible/vision-impairment friendly.

Imo we should have a light mode that people can at least shift to.

@tusharnankani
Copy link
Member

@HarshKapadia2 what do you feel about it?

@sreekaransrinath, instead of adding a light mode, do you think softening the colors but retaining the mode would help?

Either way, we'll have to modify css in all repositories.

@HarshKapadia2
Copy link
Member

HarshKapadia2 commented Oct 26, 2022

I don't mind supporting a light mode, but you're right @tusharnankani, we'll have to change the CSS, add theme switchers, check responsiveness and all of that everywhere, which will be a task.

I keep thinking that we need to build some kind of a npm package or some plugin system to be able to update and keep our UI consistent across repos. I know that Razorpay does this. It will be time consuming, but it will reduce work going down the line, I guess. I've also wanted to include Web Components for repetitive things like our burger menus, because adding one page implies changing the menu HTML on every page, which is obviously irritating (OurTechCommunity/catchup#55). (Cc: @KartikSoneji)

That being said though, I don't think we have the time to do any of these (add a separate theme or create a template) for now. Good suggestion though @sreekaransrinath. This will be an update that we will look to add in the future.

@KartikSoneji
Copy link
Member

Either way, we'll have to modify css in all repositories.

How about a page-wide invert filter for everthing except images?

@tusharnankani
Copy link
Member

tusharnankani commented Oct 27, 2022

Yep, possible.

html {
    filter: invert(1) hue-rotate(180deg);
}

img {
    /* reverse the effect for images, svgs and other assets*/
    filter: invert(1) hue-rotate(180deg); 
}

@KartikSoneji
Copy link
Member

I think we can use the :not() selector

:not(img, svg) {
    filter: invert(1) hue-rotate(180deg);
}

@tusharnankani
Copy link
Member

Best.

@HarshKapadia2
Copy link
Member

Good idea @KartikSoneji. Here is how it looks on our OTC home page. It seems okay to me for this page, but I haven't checked other pages.

Before:

image

After:

image

@tusharnankani
Copy link
Member

Too strong colours. That’s why I said — let’s soften the colours.

@HarshKapadia2
Copy link
Member

How?

@tusharnankani
Copy link
Member

I meant the colors are too strong, a little too contrasting.
And sometimes, it is difficult to read.

@HarshKapadia2
Copy link
Member

Okay, but how do we do that without adding a lot of CSS across all our repos?

@KartikSoneji
Copy link
Member

We can add a selector like:

body.invert :not(img, svg) {
    filter: invert(1) hue-rotate(180deg);
}

Then have the theme selector toggle the class:

document.body.classList.toggle("invert");

@HarshKapadia2
Copy link
Member

Yes, but as Tushar said:

I meant the colors are too strong, a little too contrasting.
And sometimes, it is difficult to read.

@KartikSoneji
Copy link
Member

invert(0.97) instead of invert(1) ?

Side note:
Maybe there can be a common css file across all repos that only defines css variables for the theme colors.

@HarshKapadia2
Copy link
Member

invert(0.97) instead of invert(1) ?

What do you think @tusharnankani?

Side note:
Maybe there can be a common css file across all repos that only defines css variables for the theme colors.

Why not have a common CSS library in that case?

@KartikSoneji
Copy link
Member

Side note:
Maybe there can be a common css file across all repos that only defines css variables for the theme colors.

Why not have a common CSS library in that case?

A common library will require a full rewrite (which is not a bad thing, just not sure if we have the time for that)
A common file with css variables will be much easier to integrate.

There should be a tool/webpack plugin to do something like that I think.
If not, it should be easy enough to build one.

@HarshKapadia2
Copy link
Member

A common library will require a full rewrite (which is not a bad thing, just not sure if we have the time for that)
A common file with css variables will be much easier to integrate.

Isn't it a temporary solution that might become permanent if we don't handle it properly? I agree that we don't have the time right now, but maybe we can start work on it and progress slowly? The light mode issue doesn't seem pressing, so maybe we can do it the correct way? I know Accessibility is important, so I'm not sure what we should do. What do you think @KartikSoneji?

Should we do both? Implement the temporary solution (which will also consume time, but less of it) and start work on the common library as well (which will consume a lot of time)?

There should be a tool/webpack plugin to do something like that I think.
If not, it should be easy enough to build one.

How would this thing work?

@KartikSoneji
Copy link
Member

A common library will require a full rewrite (which is not a bad thing, just not sure if we have the time for that)
A common file with css variables will be much easier to integrate.

Isn't it a temporary solution that might become permanent if we don't handle it properly? I agree that we don't have the time right now, but maybe we can start work on it and progress slowly? The light mode issue doesn't seem pressing, so maybe we can do it the correct way? I know Accessibility is important, so I'm not sure what we should do. What do you think @KartikSoneji?

Should we do both? Implement the temporary solution (which will also consume time, but less of it) and start work on the common library as well (which will consume a lot of time)?

Sorry I should have been more clear.
When I say you should try to avoid temporary solutions that might become permanant, I mean temporary solutions that are not the ideal way of doing things.
Whenever we start building a common library we anyways need to switch all the colors to theme variables.
Fortunately, in this case the temporary solution lends itself well to gradual adoption. We can slowly switch out parts of the site with a common framework as and when they're done.

The invert(1) filter is exactly the type of patch I try to avoid, but in this case it gets us 95% of the way there with a negligble amount of work, and is trival to roll back, so I think it's worth the tradeoff.

There should be a tool/webpack plugin to do something like that I think.
If not, it should be easy enough to build one.

How would this thing work?

  • Parse CSS files with babel
  • List all hardcoded colors
  • Replace hardcoded colors with CSS variables

@HarshKapadia2
Copy link
Member

HarshKapadia2 commented Nov 5, 2022

There should be a tool/webpack plugin to do something like that I think.
If not, it should be easy enough to build one.

How would this thing work?

  • Parse CSS files with babel
  • List all hardcoded colors
  • Replace hardcoded colors with CSS variables

That feels hacky. Won't we have to scrap this when implementing the common library?

Fortunately, in this case the temporary solution lends itself well to gradual adoption. We can slowly switch out parts of the site with a common framework as and when they're done.

How does the temporary solution lead to the final one?

@KartikSoneji
Copy link
Member

There should be a tool/webpack plugin to do something like that I think.
If not, it should be easy enough to build one.

How would this thing work?

  • Parse CSS files with babel
  • List all hardcoded colors
  • Replace hardcoded colors with CSS variables

That feels hacky. Won't we have to scrap this when implementing the common library?

No, the idea is we replace all colors with css variables.

Fortunately, in this case the temporary solution lends itself well to gradual adoption. We can slowly switch out parts of the site with a common framework as and when they're done.

How does the temporary solution lead to the final one?

As the common framwork develops, more and more site-specific css can be removed.

@HarshKapadia2
Copy link
Member

HarshKapadia2 commented Nov 6, 2022

Okay. Please propose a plan for this, @KartikSoneji.

Also, is this the correct place to discuss this or should we move this to somewhere else? If not, please propose the plan at the other place. Maybe a new issue?

@SirusCodes
Copy link
Member

@KartikSoneji IMO temporary solutions are the most permanent one.

Implementing light mode properly is what I feel is a better way to do it.

@HarshKapadia2
Copy link
Member

As mentioned before, @SirusCodes:

Fortunately, in this case the temporary solution lends itself well to gradual adoption. We can slowly switch out parts of the site with a common framework as and when they're done.

How does the temporary solution lead to the final one?

As the common framwork develops, more and more site-specific css can be removed.

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

No branches or pull requests

5 participants