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

Fixes color mode selection #12335

Closed
wants to merge 7 commits into from
Closed

Fixes color mode selection #12335

wants to merge 7 commits into from

Conversation

abhi1693
Copy link
Member

Fixes: #12330

@abhi1693 abhi1693 requested a review from jeremystretch April 24, 2023 17:01
@jeremystretch
Copy link
Member

Thanks @abhi1693! I'm going to hold off on this until v3.5, just because it touches a lot of Javascript that may be difficult to fully test across various browsers & operating systems, and v3.4.9 is the last planned release for v3.4

@abhi1693
Copy link
Member Author

abhi1693 commented Apr 25, 2023

This is supposed to be released under v3.5 only due to the reasons you have mentioned and the introduction of a new option.

@jeremystretch
Copy link
Member

I haven't dug into this, but it appears to break the ability to toggle light/dark mode using the link under the user menu (top right corner).

# Conflicts:
#	netbox/project-static/dist/netbox.js
#	netbox/project-static/dist/netbox.js.map
@abhi1693
Copy link
Member Author

abhi1693 commented Apr 27, 2023

Ok, so the functionality only works when changed via the Profile option. Via that it still works as expected. Do we need the extra options in the footer and the top-right drop-down? Because the whole idea was to get rid of local storage to avoid further issues.

I played around with the code a bit more and it requires more work if we want to keep the extra buttons for convenience. I overlooked the rack-elevation fix in my previous commit though.

@jeremystretch
Copy link
Member

Do we need the extra options in the footer and the top-right drop-down?

Yes, I believe we want to keep those, so that users have the option of easily toggling light/dark mode directly within the UI.

@abhi1693
Copy link
Member Author

abhi1693 commented May 3, 2023

I'll redo the implementation using local storage only and I think I should be able to clean up the complex logic that's in there.

@abhi1693 abhi1693 removed the request for review from jeremystretch May 4, 2023 11:03
@abhi1693 abhi1693 marked this pull request as draft May 4, 2023 11:04
@abhi1693 abhi1693 changed the base branch from feature to develop May 6, 2023 16:19
@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2023

This PR has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further action is taken.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Aug 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2023

This PR has been automatically closed due to lack of activity.

@github-actions github-actions bot closed this Sep 4, 2023
@abhi1693 abhi1693 deleted the fix/color-mode branch September 11, 2023 13:55
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pending closure Requires immediate attention to avoid being closed for inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Color mode does not take system theme in consideration after initial load
2 participants