-
Notifications
You must be signed in to change notification settings - Fork 4.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
Align the layout of the template card with the block card #35391
Conversation
Size Change: +33 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
packages/edit-site/src/components/sidebar/template-card/style.scss
Outdated
Show resolved
Hide resolved
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.
Nice simplification. I left one variable comment, but otherwise looks good to me in both editors:
Love the simplification as well.
I think there's a separate conversation on whether the indented subtext is worth it, but that's just cloudy thoughts:
I also think it's increasingly frustrating that there's so much drift between site editor and post editors, I wish we could start to componentize headers, sidebars, layout etc — and start to consume them in more universal ways.
packages/edit-site/src/components/sidebar/template-card/style.scss
Outdated
Show resolved
Hide resolved
Is there a possibility of unifying the implementation (not in terms of reusing, but more using the same primitives/components) and maybe making use of the new set of UI components instead of relying on custom CSS ? I guess I see this as a good opportunity to prove these components more. cc @ciampo (Shouldn't stop you from merging) |
(sorry wrong button) |
Probably not. But then it feels a little awkward to me if the name is the only thing that is indented, albeit by the icon, but still... something looks off to my eye. It's exploring separately I think. Perhaps as a part of the componentisation of this element. |
100%! The main question for me is more about where these components would live. Are they general enough to be in
Thank you for the ping, @youknowriad ! I agree with you, it'd be nice if we could look into using |
I was thinking that we may not necessarily need new component but just learn how to use existing ones more consistently (and fix their bugs), we have a ton of components already, |
Would adding more Storybook examples be a good way to improve the situation? |
I think it could help yes and also trying to highlight it for developers maybe in #core-editor meetings and other places in the docs. |
Sounds like a plan. @jasmussen , when you have some time (not urgent at all!), could you prepare a few examples of this pieces of UI so that we can add them to Storybook? I'm wondering what component should be best suited to host these examples — maybe |
I wonder if we could a dedicated section in storybook for the higher-level examples. |
Yes, happy to help, let's chat through this, I need to understand a few things as well. But from what I can understand, it all ties back to #27331 and the stated goal of letting developers use simple components to piece together the pieces of an inspector, without ever needing to enqueue additional stylesheets to make things line up. Take something like the block description, this one: It's a very basic layout, icon and text on the side. But it's a separate component with its own stylesheet there just to add space between the icon and the description: I would like to just write less bespoke CSS for a bunch of such small cases. Is that along the lines of what you were both thinking? |
💯 |
Currently the layout of the Template card in the Inspector does not match the Block card. This is especially apparent when you navigate between each tab.
Interestingly the template and site editor environments have different implementations. Here's an image outlining the various issues:
This PR ensures the layout and type styles are aligned with the block card: