Remove Legacy Admin Layout #2127
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We want to remove the legacy layout of the admin and this is one of the few remaining pieces that relied on a specficically named element being available.
This is a simplification of this admin interface in order to prepare to switch the admin to a responsive layout. This optimization had a weird side effect in that it fully replaced the content on the page instead of loading the form in, say, a modal. This would mean that hitting "Cancel" on the form would actually trigger a page load instead of just removing the form from the page.
|
Looks like the order page is borked. Will fix. |
|
Fixed the order page. The bug was from a few releases ago but it went unnoticed because none of the pages using the new layout also had a sidebar. |
graygilmore
force-pushed
the
responsive-admin
branch
from
6bf36c0 to
318e9fd
Compare
This commit removes the conditional in the admin layout to fallback to the legacy layout. By moving all pages to the new layout we get (relative) responsiveness for free. Since we've transitioned all of the `col-X` classes to use Bootstrap we should no longer have any fixed-width grid elements.
In a previous commit the `make-row()` mixin was removed from `.content` causing the sidebar to not have enough room on the right side of the screen. Now that we're moving to the new layout full-time we can just add the necessary Bootstrap classes directly to the markup. Special note: we're explicitly not using the `.container` class because we don't want to use Bootstrap's built-in breakpoints.
If stores are still using these classnames they can import this file directly into their project but we made the move to Bootstrap over a year ago and it's time to remove the confusion of having two grid systems.
graygilmore
force-pushed
the
responsive-admin
branch
from
318e9fd to
38fb6b8
Compare
cbrunsdon
approved these changes
Aug 1, 2017
Fix the vertical position of the content sidebar for layouts having tabs and having none.
tvdeyen
approved these changes
Aug 1, 2017
There was a problem hiding this comment.
Thanks! Was about time
I added a fix for the vertical position of the sidebar. Sent you a PR @graygilmore
Adjust sidebar to new admin layout
|
That's great! Thank you! Happy to see |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes the legacy admin layout and gives us half-decent responsiveness due to the nature of using Bootstrap classes. Previously, using
.containerour content would sit at a maximum width of960pxand shrink down to awkward sizes depending on the browser size.This PR also finally removes the Skeleton Grid CSS from the admin. If stores are still relying on it in order to make overrides to their stores they can pull that CSS directly into their app.
Oooo, fancy!
Before at awkward breakpoint
After