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

Fix switch theme #85

Merged
merged 4 commits into from
May 30, 2019
Merged

Fix switch theme #85

merged 4 commits into from
May 30, 2019

Conversation

kimulaco
Copy link
Contributor

I updated script.js for #84.
There are plans to display the status of the color scheme of the selected theme or device, but for the moment we are fixing only the switching function.

  • Switch to "standalone" after clicking "Switch theme".
  • If you change the device scheme, it will switch to the normal version.
  • Create "ThemeSwitcher" considering "version picker".

There are some suggestions for future work.

  • Add JavaScript development environment using babel.(Bundle by gulp because you already use gulp)
    • Improve code prospects.
    • Easy browser support.
  • Integration of "Version picker" and "Switch theme". (Add version picker in demo #27)
    • Status messages are integrated here.
    • There are also "standalone" and "legacy", so it is difficult for the user to understand the situation if there is only one "Switch theme".

Copy link
Collaborator

@kylejrp kylejrp left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @kimulaco!

Unfortunately, this breaks the switch button on IE11. I see a light theme by default and the button doesn't do anything.

You're right to suggest that we use something like babel to fix our browser compatibility for the docs site. We were just getting by without it before!

I'm assuming that we have more IE11 users than prefers-color-scheme users, so I don't want to merge this yet until it's working for IE11 users too. Are you able to write IE11 compatible Javascript instead? (It's not fun, I'm sorry!)

If you need any help, please let us know! I've seen some great cross browser testing tools suggested here. I'm not sure what OS you're using, but I can test IE11 on mine.

@kylejrp
Copy link
Collaborator

kylejrp commented May 30, 2019

Hmm, actually, the button is broken on IE11 for the normal https://watercss.netlify.com/. I'm not sure what broke this.

Since IE11 is broken unrelated to your merge request, I think we can just merge this in. Sorry for the scare!

@kylejrp kylejrp merged commit 03269b4 into kognise:master May 30, 2019
@kimulaco
Copy link
Contributor Author

@kylejrp Thank you for checking!
I will make another PR and add babel for the future.
And I will try the correspondence of IE.

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.

None yet

2 participants