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.

Sign up for GitHub

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

Jump to bottom

PL: rather plane application banner #19995

Merged
merged 1 commit into from Jan 13, 2018
Merged

PL: rather plane application banner #19995

merged 1 commit into from Jan 13, 2018

Conversation

breville
Copy link
Member

Desktop:
screenshot 2018-01-12 16 00 32

Mobile:
screenshot 2018-01-12 16 00 43

@breville breville requested a review from Erin007 January 13, 2018 00:04
Erin007
Erin007 approved these changes Jan 13, 2018
View reviewed changes
Copy link
Contributor

@Erin007 Erin007 left a comment

Choose a reason for hiding this comment

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

Super cute! Are we trying to move away from inline styles? Otherwise, LGTM!

@breville breville merged commit 5d8f778 into staging Jan 13, 2018
@breville breville deleted the professional-learning-apply-banner branch January 13, 2018 00:10
@breville
Copy link
Member Author

I have a range of feelings, but am amused that React took us way towards inline styles after previous efforts. This banner was produced very quickly and I'm not sure how long it'll survive, so it's going in as it was created. I could pull styles out but I think it does the job nicely for today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Erin007 Erin007 Erin007 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@breville @Erin007