File block: Try refreshing styles #39118
Conversation
jasmussen
added
[Type] Enhancement
|
Size Change: +641 B (0%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Thanks for exploring this Joen!
While this looks good, it will affect all existing File blocks. Using flex sounds good, but adding a border in front end where it didn't exist seems a rather big opinionated change to me. Do you think it will still be an improvement without the border?
I'm not sure, no, I think it needs to be a "card" of sorts lest the change to have link left and download button on the right just spaces them apart. Perhaps the PR needs to be combined with adding border and background support? |
|
|
||
| // The file block already has a border, so don't show the extra border of the placceholder. | ||
| .components-placeholder.components-placeholder { | ||
| box-shadow: none; |
There was a problem hiding this comment.
I suppose if we do add border support, you could set the border to zero which might be weird in the placeholder state 🤔
Then again, it seems like the placeholder state itself could do with a great deal of iteration to be more minimal — almost just a small dashed-line dropzone of the same size as the resulting block? Probably best to explore separately.
|
This is how it looks in Livro when I remove the file block CSS from the theme: This avoids us needing to style the button, although with those styles there it still looks fine:
I think we should move forward with this approach but I'm curious what @kjellr and @beafialho think... |
| @@ -29,16 +23,27 @@ | |||
| left: 0; | |||
| } | |||
|
|
|||
| // This is an extra container present only in the editor view. | |||
There was a problem hiding this comment.
Do we still need to have different markup in the editor?
There was a problem hiding this comment.
Probably not. The block feels somewhat oldish. Feel free to commit to the PR if you like!
There was a problem hiding this comment.
Might be better as a follow up!
I think these options look good. @scruffian just to understand why the screenshots are different, is the first one the default and the one below styled? |
The top one is what it should look like without any custom CSS. The bottom one is what it looks like with the custom CSS that we have in place to "fix" the old block. I think we should remove that CSS but until we do so it doesn't look broken anyway :) |
|
A bit of context on the "Button" style differences: the old block was designed as if the Download link looked like the Button block. But it wasn't a button, and it would be styled differently. This PR rethinks the role of the download button to be just an explicit separate link, where the whole "card" (hence the border) is the file. It's a bit of an opinionated change, thus the trickiness in landing this as is. Thank you all for looking! |
I agree 👍 |
|
The remaining concern here is the impact that this will have on existing blocks. Do you think we need a way to mitigate this and ensure this change only applies to new blocks? |
It's an excellent question. On the one hand, it's a big enough visual change to consider it. On the other hand, the older design was lacking in so many ways there's an argument to be made this falls in the category of a bugfix. If we were able to find some existing content (theme plus use of the block) in the wild, see how it would be affected by this, it might help inform a decision. |
|
Here are some examples across some popular themes: Intergalactic 2 Canard Hever Twenty Twenty One I think the 2 main things are
Curious what @kjellr thinks... |
Yes, that's a good summary. The inspiration for both of these, is this design: The border and the background makes this feel like a file. Filename on the left, download button on the right, a single unit. We can't adopt that design wholesale, at least yet, the main challenge being finding colors that are agnostic across themes and working in dark contexts (hence the liberal use of Given that's the original motivation, are there ways to move towards that? Is this PR a good step towards it? Given all the testing (thank you!) I personally feel more confident that this is a nice iteration of the block, and that next steps would be to add border and background controls to the block. |
|
I think the design here could use a little bit of iteration. I like the reference, but I think the outline we're using in the PR feels a bit bland? I know we're aiming to keep it un-opinionated, but it currently looks like something that a theme would be required to style in order to make it feel like a considered part of the UI. I think it would be worthwhile to explore some slightly more opinionated versions of the block. Some potential ideas:
|
|
Excellent thoughts, I echo them, and in fact I explored a few of them as part of the PR. What ultimately made me pause was for the same reason Ben mentioned, it's been in a bad state for so long, that even tiny changes might make ripples, and I was hoping to take it in small steps. That said, it might be worth having more of a "vision" mockup for where this could go, rather than go by a mental image mainly.
I like this idea a fair bit! Can you clarify what you mean by an SVG background, though? Ideally we find a way to add a currentColor background that can then trivially be changed using theme.json and background tools, such as this: The problem with currentColor is (barring any SVG tricks you have in your sleeve), it has to sit in an abs-positioned pseudo element in order to function, right? I wonder if we can use |
I'd been hoping that SVG would let you use |
|
I like a challenge. I'm going to marinate on how we can add a semi opaque currentcolor somehow, and see if I can figure something out. I'd love to avoid a pseudo element if possible! |
|
I tried a number of things, including backdrop-filter, and filter/opacity, but nothing that worked. The closest I got was an inner box-shadow set to current color, where the blur applied did actually add some opacity. But of course it appeared as a halo inside. So I'm not sure how best to proceed here. It may need a design re-think? |
|
Yeah, maybe we should take a step back, mock up a few options for what we want, and then figure out which of them we can build. |
|
Oh, one idea that inspires a question: can we make a new default style variation, but keep existing content as style variation two? Then we could potentially make the new default more opinionated, but keep the old one around. |
|
I think that sounds reasonable. 👍 |
|
Alright, I took a look at this one today, and as it turns out, there is a path forward as outlined above using block styles. Let's say we created "Default" and "Plain" to match Quote names; default would be this new bordered version, and would attach to Cool! However, this recent comments on block styles reminded me that we mean for most block styles to be absorbed by theme.json and design tools. I.e. instead of having a Would style variations set CSS classes in that case? My instinct is no, but I'd love to hear your insights if you have any, CC: @scruffian. In the mean time, I'm going to let this one sit while I extract some of the safe changes to a separate PR. |
|
Maybe we should wait to see what happens with the move of block styles to theme.json... |
|
Yep, that's my instinct too. I'm going to close this one for now, and we'll revisit! Thanks for all the good thoughts shared. |
Description
The file block default visuals are a bit out of date:
Although basics can be styled using theme.json, it's not very dark mode or theme aware out of the box. There are also missing focus styles and some baked in very opinionated styles we arguably no longer need. This PR does a few things:
After:
After in TT2:
The new style should also make it vastly simpler to bring various block supports, such as border, radius, font and more to the block.
Although this fixes and tweaks a few things, it does change a style that has been default for a long time. I'd love your thoughts on the impact of this. I personally think it's worth it, given the benefits it brings, and how simpler it is to style.
Testing Instructions
Please test the file block with and without a separate download button, with and without a PDF preview (see also #30857), with the various alignments available, with a few themes, and on mobile.
Checklist:
*.native.jsfiles for terms that need renaming or removal).