-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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
Framework: Allow for a sans-sidebar layout and adjust accordingly #302
Conversation
I should make note that this will affect the |
I checked /plugins and /settings and a few other places and notices all look good from my checks. |
I added the |
@@ -93,6 +94,10 @@ module.exports = React.createClass( { | |||
'is-active': this.state.isLoading | |||
} ); | |||
|
|||
if ( this.state.noSidebar ) { | |||
sectionClass += ' sidebar-off'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should write this one as is-sidebar-off
for good measure.
I'll pick this up after launch. |
|
Can I get the Translator Invitation on the radar here too? You can see it by setting It looks like this PR already fixes it, so hopefully it's no extra work (right now it's under the sidebar #469 ). |
Not all the elements on the screen live inside .wp-primary. Things like notices are children to the primary element. This causes some hassles with the new sidebar layout. To make it easier, lets apply our layout styles to .wp-content. Along the way I moved the sidebar width to a variable.
74d37fa
to
ef709e7
Compare
This avoids the misaligned dismiss button mentioned in #214 Removing the unnecessary adjustments on <Notice> for the sidebar — this is all handled by the notice’s parent element .wp-content. Also fixed .is-pinned to work with the new margins and widths. For some reason, I can’t use the $sidebar-width-min/max SCSS variables here.
Use the new noSidebar layout state on the welcome screen. Making the new noSidebar state actually do something — in this case, it removes the extra padding thats not needed without the sidebar. Switching everything to use to new noSidebar layout. Making sure the padding on wp-content is 8px on mobile.
This PR aims to address two things:
These are bugs that were introduce with the recent docked sidebar update, so I'm trying my best to get them fixed.
The solution to broken notices was to move all the layout-related CSS from
.wp-primary
to.wp-content
— which contains notices. This means no more funky layouts when you see a notice.Once I made that change (and after hearing a few reports of busted layouts) I noticed that notices were still broken on screens without a sidebar. Moreover, screens without a sidebar were busted in general. So, I worked to fix the broken layouts (and notices in the process) by creating a new state for layout:
context.layout.setState( { noSidebar: true } );
will now remove any extra padding/margin so you don't have to. This should replace any negative margins or the.is-full
class.You can check the notices by submitting a blank payment form at
/checkout
or (if you're like me) looking at a Jetpack site that has connection issues.The noSidebar stuff is scattered throughout, including
/login
,/me/next/welcome
,/checkout/thank-you
, '/devdocs, and
/sites`.