Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Handle undefined theme the right way#10243

Merged
MiguelCastillo merged 1 commit intoadobe:masterfrom
marcelgerber:theme-undefined
Dec 20, 2014
Merged

Handle undefined theme the right way#10243
MiguelCastillo merged 1 commit intoadobe:masterfrom
marcelgerber:theme-undefined

Conversation

@marcelgerber
Copy link
Copy Markdown
Contributor

Sorry for the confusion.
This is the right way to do #10236.

Comment thread src/view/ThemeManager.js Outdated
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This has never caused any issues, has it?
cc @MiguelCastillo

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@marcelgerber no we haven't ran into issues. That's because we have this https://github.com/adobe/brackets/blob/master/src/view/ThemeSettings.js#L49.

That discrepancy was introduced in the shuffling when we were trying to land themes. I have cleaned up lots of that stuff in this PR #9069 which I think we should really merge, but have not heard much feedback on.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, reverted the change in this PR (as it's not necessary) to not cause any unnecessary conflicts with your PR.

@marcelgerber
Copy link
Copy Markdown
Contributor Author

@MiguelCastillo Feel free to merge this :)

MiguelCastillo added a commit that referenced this pull request Dec 20, 2014
Handle undefined theme the right way
@MiguelCastillo MiguelCastillo merged commit 4971be5 into adobe:master Dec 20, 2014
@marcelgerber marcelgerber deleted the theme-undefined branch December 20, 2014 16:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants