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

Themes: Update styling for sheets #5528

Merged
merged 4 commits into from
May 25, 2016
Merged

Themes: Update styling for sheets #5528

merged 4 commits into from
May 25, 2016

Conversation

seear
Copy link
Contributor

@seear seear commented May 23, 2016

Prerequisite for Live Preview #5482

Before
screen shot 2016-05-23 at 17 57 22

After
screen shot 2016-05-23 at 17 58 30

Design Reference
screen shot 2016-05-23 at 11 45 38

@folletto: feel free to tweak as necessary :)

@seear seear added [Feature Group] Appearance & Themes Features related to the appearance of sites. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR labels May 23, 2016
@seear seear added this to the Themes: Showcase M4-ThemeSheets milestone May 23, 2016
@seear seear self-assigned this May 23, 2016
@seear seear force-pushed the fix/theme-sheet-styling branch from 530b52b to 1348bf8 Compare May 23, 2016 17:23
@seear seear mentioned this pull request May 24, 2016
Merged
@folletto
Copy link
Contributor

In 2534a4baddfa772db15381c95370ef6d12c758de changed the CSS to allow any image height (ready for full-height screenshots).

@folletto
Copy link
Contributor

In f81a9a6a1eb234ebfca1a1a5fd2ae2cea27dc803 refactored @include breakpoint to be one in each sub-component for ease of maintenance.

@folletto
Copy link
Contributor

folletto commented May 24, 2016 •
edited
Loading

In fd32fe60e01de54c963ccd20cfc941938f217c63:

  • Added gradient.
  • Restricted height so taller images won't make the page impossibly long.

screen shot 2016-05-24 at 19 21 38

Tested on Chrome, Safari, Edge.
Couldn't test on Edge/IE due to #5495.

PR is good for me.
Let's do a final code review and merge? :)

@folletto folletto removed the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label May 24, 2016
@seear
Copy link
Contributor Author

seear commented May 25, 2016

Ok, this is looking really good now, good to merge 👍

@seear seear force-pushed the fix/theme-sheet-styling branch from fd32fe6 to 53a84e6 Compare May 25, 2016 10:23
@seear
Copy link
Contributor Author

seear commented May 25, 2016 •
edited
Loading

Just noting that either the length of the text, or the latest <HeaderCake/> causes the 'All Themes' text not to show on mobile. This matches the design reference anyway.

screen shot 2016-05-25 at 11 24 12

@seear seear merged commit 95356cf into master May 25, 2016
@seear seear deleted the fix/theme-sheet-styling branch May 25, 2016 10:30
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label May 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@seear seear

Labels
[Feature Group] Appearance & Themes Features related to the appearance of sites.
Projects
None yet
Milestone
Themes: Showcase M4-ThemeSheets
Development

Successfully merging this pull request may close these issues.
3 participants
@seear @folletto @matticbot