- πΊπΈUnited States danflanagan8 St. Louis, US
Hi @jnlar,
I finally found some time to look at this again and actually think about carefully again. I want to summarize where we are here.1. The modification to the template is all that is required to allow the Messages block to be managed within LB.
2. The modification toSystemMessagesBlock::build()
is a UX enhancement for when "Show content preview" is checked.I might argue that we should defer #2 to a follow-up. The reason I say that is that it might be worth getting a usability review. It's not obvious to me what is the most desirable UX. Here are the two version of preview UX.
A) With the update to SystemMessagesBlock::build() in #93
"Show content preview" unchecked
"Show content preview" checked, no active status message
"Show content preview" checked, with active status message
B) No modification to SystemMessagesBlock::build()
"Show content preview" unchecked
"Show content preview" checked, no active status message
"Show content preview" checked, with active status message
Both of these approaches allow the Messages block to be managed in LB. Remember, the change to the template is all that's needed to restore contextual filters and the attributes required by LB. Changing or not changing SystemMessagesBlock::build() is all a question of preferred UX.
In my opinion, Approach A is probably more usable, but Approach B gives a more-realistic content preview, which is one of the goals of LB. So I don't know which way to go!
If we were to take Approach A and keep moving with the patch in #93, I would say the test coverage needs to check the "Show content preview" box at some point and confirm that the placeholder text remains. During content preview is where the placeholder text is really supposed to appear. I don't think it's quite a bug that the placeholder text appears even when the content preview box is unchecked, but that's different from how the "Links" field behaves, for example.
I'm going to leave this at NR for now.
- π¦πΊAustralia jnlar Sydney, Australia
hi @danflanagan8,
Thanks for the summary, that'll also help bring newcomers up to speed.
I agree, the issue defined in the IS is not being able to manage the Messages block in LB. It isn't obvious to me whether or not #2 is within the scope of the IS.
I'm for deferring #2 with a follow up, more eyes on this would be good.
In my opinion, Approach A is probably more usable, but Approach B gives a more-realistic content preview, which is one of the goals of LB. So I don't know which way to go!
It does give a more realistic content preview, but you can argue that a realistic preview isn't needed for the Messages block in LB, AFAIK every other preview in LB is using placeholder content.
I don't think it's quite a bug that the placeholder text appears even when the content preview box is unchecked, but that's different from how the "Links" field behaves, for example.
I don't think so either, the olivero theme is responsible for that behaviour. The theme wraps a blocks
{{ content }}
in a div with theblock__content
class. When "Show content preview" is unchecked it'sdisplay
is set tonone
. - Status changed to Needs work
over 1 year ago 7:27pm 21 February 2023 - πΊπΈUnited States smustgrave
In addition
Not sure if this one block deserves it's own test instance.
Could this be added to testLayoutBuilderUi