Further improve columns block. #11659
Conversation
|
@youknowriad I would appreciate thoughts on whether this should be part of the 4.3 milestone or not. The only "controversial" change in this PR, is the changing of the appender from input to textarea. What do you think? |
First impression, I think it's fine because we don't really type in it. |
|
Cool. I'd really like this in 4.3 as it complements the columns PR well. Going to restart the test again, because they pass locally. |
jasmussen
force-pushed
the
update/columns-initial-state-polish
branch
from
0aea826 to
8498062
Compare
|
Adding to 4.3 milestone because it also ensures the placeholder text fits on mobile (it's longer as of #11653). |
aduth
force-pushed
the
update/columns-initial-state-polish
branch
from
8498062 to
7e01119
Compare
This PR is a followup to thoughts in #11620 (comment). It improves the columns block further by doing two things: 1. It hides the hover effect on the individual _column_ block in the initial state. 2. It changes the non-block "fake" appender to be a textarea instead of an input. To elaborate on 1, in #11620 we used pointer-events to disable the individual column blocks as selectable using the mouse. This is because those columns are not actionable at the moment, and in the current implementation of the column, being able to hover and select them only causes issues with selecting the parent columns block, which _is_ actionable. However pointer events was not enough for the _empty_ state of a columns block. In this state, the block features no inner blocks, except the individual column blocks, because no content has yet been inserted yet. In this state, you see the "appender", which is not actually a block even though it looks exactly the same as an empty paragraph. Because this is not technically a block, the deepest nested child is a _column_, which is then allowed to be hovered. In 1 I add a workaround to that, to simply visually hide the hover effect. I expect this to be refactored in a future columns update, but for the time being it makes it far more usable. To elaborate on 2, there has been a long time issue with the appender (again, not the block, the fake version that we use before an empty paragraph is inserted) wrapping text in translations. This is because text _cannot_ wrap in an `input` field, which the appender is. By changing this to a `textarea`, the text can wrap. This might affect themes which specify the input element for increased specificity in their editor styles, but overall it still seems like a worthwhile change to make, since it's the only way we can allow the writing prompt to wrap its text.
aduth
force-pushed
the
update/columns-initial-state-polish
branch
from
7e01119 to
abf541d
Compare
|
Rebased to resolve conflict. The change to
|
|
Hmm, good catch. Taking a look. |
|
Might be something which could be fixed by using |
|
I'm just about to push a fix which simply uses overflow: hidden, that seems to fix it. Any objections to using that? |
|
@jasmussen You might also try a |
|
Hmm, yes, resize: none definitely needs to be there, and "overflow: hidden" is only a partial fix. Looking more into it. |
Props @aduth. Also adds resize: none;, props @chrisvanpatten.
|
Seems to me |
|
Correct, both of you, and both of your suggestions make for the correct fix:
That's a columns block inside a columns block. And it's not even as horrible as you'd think that would be. (Not that I'd recommend anyone ever do this) |
|
Not sure why the tests are suddenly failing. Tried restarting once. Now running them again locally. They're a bit slow on my old "has a good keyboard still" Macbook. |
|
Tada! Thanks Riad for the unit test fixes. Merging when the checks pass. |
Just so you know, nesting columns like this actually comes up surprisingly often in page building. At some point in phase 2 you should be able to do stuff like this without running into any weirdness. |
This PR is a followup to thoughts in #11620 (comment).
It improves the columns block further by doing two things:
It hides the hover effect on the individual column block in the initial state.
It changes the non-block "fake" appender to be a textarea instead of an input.
To elaborate on 1, in #11620 we used pointer-events to disable the individual column blocks as selectable using the mouse. This is because those columns are not actionable at the moment, and in the current implementation of the column, being able to hover and select them only causes issues with selecting the parent columns block, which is actionable.
However pointer events was not enough for the empty state of a columns block. In this state, the block features no inner blocks, except the individual column blocks, because no content has yet been inserted yet. In this state, you see the "appender", which is not actually a block even though it looks exactly the same as an empty paragraph. Because this is not technically a block, the deepest nested child is a column, which is then allowed to be hovered.
In 1 I add a workaround to that, to simply visually hide the hover effect. I expect this to be refactored in a future columns update, but for the time being it makes it far more usable.
To elaborate on 2, there has been a long time issue with the appender (again, not the block, the fake version that we use before an empty paragraph is inserted) wrapping text in translations. This is because text cannot wrap in an
inputfield, which the appender is. By changing this to atextarea, the text can wrap.This might affect themes which specify the input element for increased specificity in their editor styles, but overall it still seems like a worthwhile change to make, since it's the only way we can allow the writing prompt to wrap its text.
Screenshots:
GIF: