Skip to content

chore: store dev-app dark/light theme state in localStorage #18218

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

Merged
merged 1 commit into from
Aug 12, 2020

Conversation

Splaktar
Copy link
Contributor

@Splaktar Splaktar commented Jan 19, 2020

It's a pain to debug components' dark theme when every time you make a change the site reloads and goes back to light theme. This just makes it remember the previous value when the dev-app loads.

@Splaktar Splaktar added the area: dev-infra Issue related to internal project infrastructure label Jan 19, 2020
@Splaktar Splaktar requested a review from andrewseguin January 19, 2020 15:20
@Splaktar Splaktar requested a review from mmalerba as a code owner January 19, 2020 15:20
@Splaktar Splaktar self-assigned this Jan 19, 2020
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 19, 2020
@Splaktar Splaktar force-pushed the dev-app-theme-local-storage branch 2 times, most recently from 9dac25b to 09f0c5f Compare January 20, 2020 15:48
@Splaktar Splaktar force-pushed the dev-app-theme-local-storage branch from 09f0c5f to bd17f9a Compare January 20, 2020 19:33
@Splaktar Splaktar added pr: merge safe target: major This PR is targeted for the next major release target: patch This PR is targeted for the next patch release and removed target: major This PR is targeted for the next major release labels Jan 20, 2020
@Splaktar Splaktar force-pushed the dev-app-theme-local-storage branch from bd17f9a to 61913b6 Compare January 20, 2020 20:04
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM once the comments are addressed.

I kind of agree with Kristiyan, who proposed that we just have the code in the same class DevAppLayout (like before). I don't mind too much though.

@Splaktar Splaktar force-pushed the dev-app-theme-local-storage branch from 61913b6 to 68cbd50 Compare January 21, 2020 14:32
@Splaktar Splaktar requested a review from wagnermaciel July 20, 2020 18:07
@Splaktar Splaktar force-pushed the dev-app-theme-local-storage branch from 68cbd50 to 54daf9a Compare July 20, 2020 18:31
@Splaktar
Copy link
Contributor Author

have the code in the same class DevAppLayout (like before).

I've done this after merging in the latest changes and resolving merge conflicts.

@Splaktar Splaktar added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Jul 20, 2020
@Splaktar Splaktar removed the request for review from andrewseguin July 20, 2020 18:34
@Splaktar Splaktar removed the request for review from crisbeto July 27, 2020 22:02
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

LGTM

@mmalerba mmalerba added the action: merge The PR is ready for merge by the caretaker label Aug 10, 2020
@andrewseguin andrewseguin added P2 The issue is important to a large percentage of users, with a workaround and removed P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Aug 10, 2020
@andrewseguin andrewseguin merged commit 0665d86 into master Aug 12, 2020
andrewseguin pushed a commit that referenced this pull request Aug 12, 2020
@Splaktar Splaktar deleted the dev-app-theme-local-storage branch August 12, 2020 08:54
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: dev-infra Issue related to internal project infrastructure cla: yes PR author has agreed to Google's Contributor License Agreement P2 The issue is important to a large percentage of users, with a workaround target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants