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

Added handler for dark mode #19604

Closed
wants to merge 1 commit into from
Closed

Added handler for dark mode #19604

wants to merge 1 commit into from

Conversation

Aelof3
Copy link

@Aelof3 Aelof3 commented Jun 9, 2020

  • Added style block to head via javascript
  • Added listener to update color scheme when changed in parent

- Added style block to head via javascript
- Added listener to update color scheme when changed in parent
head.appendChild(style);

if ( style.styleSheet ){
// This is required for IE8 and below.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need to support IE8.

@looeee
Copy link
Collaborator

looeee commented Jun 10, 2020

Related: #19603

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 10, 2020

What is the relation between this PR and #19603?

@Aelof3
Copy link
Author

Aelof3 commented Jun 10, 2020

this modifies index.html to modify the color scheme, and sends a ping to the page.js in #19603 where it updates the color scheme for the doc loaded in the viewer iframe

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 10, 2020

Doesn't it make more sense to bundle everything into a single PR? It's very unusual to create a PR for each modified file.

@Aelof3
Copy link
Author

Aelof3 commented Jun 10, 2020

gonna be honest, this is my first change to a repo other than my own and I have no idea how to do that

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 10, 2020

How have to created the PR? By forking the repo on you computer or via the GitHub UI?

@Aelof3
Copy link
Author

Aelof3 commented Jun 10, 2020

Used the edit file button on the GitHub UI, which forked it for me I am pretty sure

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 10, 2020

It's just important that you change all files in the same branch. Then you can submit a single PR with all changes.

I suggest you study a bit the contribution guide and try it again:

https://github.com/mrdoob/three.js/blob/dev/.github/CONTRIBUTING.md#contribution

One major problem right now is that's in not possible to test your changes. When changes are split over PRs, there is no convenient way to verify what you do.

@Aelof3
Copy link
Author

Aelof3 commented Jun 10, 2020

I see, I will close this and the other one and see what I can do

@Aelof3 Aelof3 closed this Jun 10, 2020
@Aelof3 Aelof3 deleted the patch-2 branch June 10, 2020 10:11
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.

3 participants