Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

Welcome dark theme #487

Merged
merged 3 commits into from
Jul 23, 2019
Merged

Welcome dark theme #487

merged 3 commits into from
Jul 23, 2019

Conversation

rossmoody
Copy link
Contributor

@rossmoody rossmoody commented May 31, 2019 •
edited
Loading

@rossmoody rossmoody changed the title WElcome Welcome dark theme May 31, 2019
@rossmoody rossmoody requested review from cezaraugusto and imptrx May 31, 2019 05:20
@rossmoody rossmoody self-assigned this May 31, 2019
@rossmoody rossmoody requested a review from petemill May 31, 2019 05:28
cezaraugusto
cezaraugusto reviewed Jun 4, 2019
View reviewed changes
Copy link
Contributor

@cezaraugusto cezaraugusto left a comment

Choose a reason for hiding this comment

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

code looks good but I think we should tweak the secondary button to fit better on dark mode:

Skjermbilde 2019-06-04 kl 17 08 04

@cezaraugusto
Copy link
Contributor

also on hover it almost disappears

@rossmoody
Copy link
Contributor Author

rossmoody commented Jun 5, 2019 •
edited
Loading

@cezaraugusto I rebased and this one is worth another look. I think I was working on the button PR at the same time but now that it's merged that button seems to be ok.

This is what I'm seeing:
image

I think implementing this on core side with a theme provider is a little out of my depth at the moment. Any way you could help with that?

cezaraugusto
cezaraugusto approved these changes Jun 21, 2019
View reviewed changes
Copy link
Contributor

@cezaraugusto cezaraugusto left a comment

Choose a reason for hiding this comment

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

++ please open a PR in brave-core with these changes before merging as I believe we have some breaking changes here. thanks Ross!

@imptrx
Copy link
Contributor

imptrx commented Jun 22, 2019

we are currently waiting on some updates on the brave-core side to enable dark mode work on our internal pages. I can help with the core PR once it's ready as I'll be doing analogous work for NTP

@rossmoody rossmoody mentioned this pull request Jun 25, 2019
Closed
@imptrx imptrx mentioned this pull request Jul 10, 2019
Merged
9 tasks
@bsclifton bsclifton mentioned this pull request Jul 23, 2019
Merged
32 tasks
@imptrx imptrx merged commit 4e80f1d into master Jul 23, 2019
@petemill
Copy link
Member

We'll need to import this to Brave-Core directly since Welome-specific UI is no longer used from this repo as of brave/brave-core#2896

@imptrx
Copy link
Contributor

imptrx commented Jul 24, 2019

I'll address all the changes here in brave/brave-core#2909 😃

@rossmoody rossmoody deleted the welcome-dark branch July 24, 2019 22:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cezaraugusto cezaraugusto cezaraugusto approved these changes

@imptrx imptrx Awaiting requested review from imptrx

@petemill petemill Awaiting requested review from petemill

Assignees

@rossmoody rossmoody

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rossmoody @cezaraugusto @imptrx @petemill