-
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
[WIP] Add block elements interactivity states #41383
Conversation
Size Change: +1.07 kB (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
Do folks have thoughts on how (or even if) we'd style elements and blocks using pseudo-classes in the future? I'm mainly thinking of whether we'd want to open this up to more elements and properties under "h1:hover": {
"color": {
"text": "red",
"background": "blue",
},
"border": {
"width": "1px",
"color": "green",
},
} This could complicate the UI further, granted. Some of the design proposals over in the navigation issue appear instructive. |
@ramonjd I think if the Similarly, in the UI we'll probably not offer all the options. It depends on how unintrusive the result will be. Some of the proposals from #38277 (comment) in #38277 are well suited for a big number or things supporting "states". |
@ramonjd Basically I agree we will want to support pseudo classes on more elements. I don't think we'll look at this on elements which by default are not focusable or interactive (e.g. I did originally try to refactor this PR to support more elements and not just the |
506476c
to
9435626
Compare
Thanks for the info @getdave
Ah ya, good call. I just pulled
Fair enough, my interest lies above all in the model, the I think so long as the sub-properties don't deviate from our current style tree model things will be dandy. By "sub-properties" I mean everything under "link:hover": {
"color": {
"text": "red"
}
} |
9435626
to
0067052
Compare
Thanks for the explanation. The style engine will have to take care of dealing with the model, but I get the feeling that processing css for a single element at once will be preferable, rather than treating Especially if we ever want to consolidate styles rather than print To me having additional pseudo classes as top-level elements doesn't fit well with the concept of a "elements", rather they are states of elements. So if we decide to support
I feel you! 😄 My go-to model would be I suppose I'm just trying to imagine:
Sorry if I'm throwing sticks in the spokes. I agree it's very complex. |
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 work progressing this forward @getdave and @draganescu! It'll be great to add in support for pseudo classes, and finding a minimal path forward is a really good idea, as the discussion can get complex quite quickly. I love the designs in #38277 (comment), but I agree in the shorter-term, coming up with a simpler UI / living with a bit of duplication, sounds like a pragmatic first step.
I think before getting there, though, while we can continue to iterate on the UI, it'd be good to come up with a data model that we feel will be unlikely to change, and will make it easier for us to extend and add support for both additional elements / blocks, and additional pseudo classes and other features. The goal would be to ensure that when we go to extend the feature, the data in block attributes is already in a friendly shape to extend (so we can avoid having to migrate block attributes when we go to make changes).
For example, rather than creating a new element of a:hover
, what if we allowed additional levels of nesting for pseudo-classes or other selectors:
"link": {
"color": {
"text": "red"
},
"selectors": {
":hover": {
"color": {
"text": "blue"
}
}
}
}
With the above, we'd then defer to the style engine to iterate over the selectors or pseudo classes, to then output the correctly concatenated rules.
Some of the things I think we'd be likely to want to explore in the future could include:
- Adding a
transition
prop so that hover states can be faded / transitioned to — I think it might make more semantic sense for these sorts of related values to all be within the one "element" rather than treating the pseudo-class state as a separate element - Implementing other pseudo-classes, for example
:disabled
on form fields, or visited links, etc
One potential benefit of using a child selector
key is that it could also be something generic that might help get us closer to supporting blocks with complex color requirements (e.g. it could potentially open up being able to target different elements of the Table block for individual color controls, but that's a little off topic to this PR 🙂)
What do you think?
I'm attracted to this idea, platonically for now, but who knows.... 😄 It would inform us that there are additional sub-properties available without having to walk through the entire element object. Nice! |
If we're going for nested syntax, I think we should be explicit and simple and let the system work it out. By that I mean the syntax would be more in line to:
No colon, states instead of selectors. |
That sounds like a good path forward to me, @draganescu! I quite like That nested approach also plays nicely with a long-term wishlist idea of a
Happy to help review any follow-ups! |
I've had rough stab at outputting the elements + states css model proposed above over at #41619 |
Catching up here after various AFK and other ongoing work distracting me. I like the new data model. It's instructive to see how proposing a simple option upfront has allowed us to visualise the flaws with the approach and iterate towards the nested We can probably close out this branch at this point - what do you think @draganescu? |
Closing in favour of #41708 |
What?
Exploratory PR to add basic interactivity states to links via the block UI. Builds on great work from @draganescu in #41331 and long term it will look to close #27075 (or at least address some part of it) and ultimately also #38277.
Why?
We need to explore whether the proposed elements syntax of
...is going to be the most appropriate. We also need to stress test the limitations of the current system to check how much work is involved in getting this feature to land.
How?
This is currently a lot of copy/paste to get
:hover
state on a link using the UI).Currently it's very basic and we'd need to do some refactoring to make the code more generic and robust.
Testing Instructions
:hover
.Screenshots or screencast
Screen.Capture.on.2022-05-26.at.20-54-58.mov