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

Adds dark class and moves inline-widget color into core UI theme #8502

Closed
wants to merge 1 commit into from

Conversation

dangoor
Copy link
Contributor

@dangoor dangoor commented Jul 22, 2014

See this comment from #8484.

cc @MiguelCastillo to review the ThemeManager change and @larz0 to make sure that the new location for the .inline-widget style makes sense.

@MiguelCastillo
Copy link
Contributor

@dangoor
Copy link
Contributor Author

dangoor commented Jul 22, 2014

Ha! I was going to review that PR next. I should have done it first. Can we move the .inline-widget change to your PR and close this one?

@MiguelCastillo
Copy link
Contributor

Yeah man, either way. It seems a bit better to just remove my one liner and merge in your change because this PR is more specific. I like my little toggleClass logic though :D

@dangoor
Copy link
Contributor Author

dangoor commented Jul 22, 2014

Yeah, toggleClass is nicer.

* These are overrides to make the UI look better when the editor theme is dark.
* Eventually, these will likely be replaced by full-featured UI themes.
*/
.dark .inline-widget {
Copy link
Contributor

Choose a reason for hiding this comment

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

To be future-proof, we should do it like this:

.dark {
  .inline-widget {
    color: #333;
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I will probably do that this afternoon as I try to bring in some dark styling to the core UI.

@larz0
Copy link
Member

larz0 commented Jul 22, 2014

@dangoor the new location makes sense 👍

@dangoor
Copy link
Contributor Author

dangoor commented Jul 22, 2014

Thanks @larz0. Closing this in favor of #8505 (which takes care of this as well as the scrollbars)

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.

4 participants