At this moment, blocks using “Block Context” only show the context being used when more than one context is available.
This is confusing for the end-user.
This is a follow-up from #2377757: Expose Block Context mapping in the UI see remarks in #65, #165 and #170. In the original patch there was a solution that added a label with a "context_slot_title". This was removed in https://www.drupal.org/node/2377757#comment-10365663 but never replaced with a something that actually shows the context with > 1 option.
For FacetAPI we are using the block context system. At this moment there is no way to see the facet name being placed by the block, if only 1 facet exists, which is very confusing and requires extra documentation or a form_alter on the block edit form, while this can easily be fixed in core.
As far as I know, the context system is not in use yet by any core feature as long as #2550199: Add a ContextProvider for Feeds is not in yet. This means there is no disruption at all. Edit: At least since #2339151: Conditions / context system does not allow for multiple configurable contexts, eg. language types, the context system is used in core.
Visual changes
Currently in HEAD:
With patch applied:
Why this should be an RC target
- Without this patch, there is no mention of the context in the UI with only 1 context created, this is confusing for the user.
- There is no disruption or change to current core behaviour, because this is not yet in place for anything in core.
- There was similar functionality in core before this originally got comitted.
Comment | File | Size | Author |
---|---|---|---|
#34 | show_context_in_blocks-2603454-34.interdiff.txt | 751 bytes | Arla |
#34 | show_context_in_blocks-2603454-34.patch | 2.37 KB | Arla |
#23 | show_context_in_blocks-2603454-22.patch | 2.36 KB | Arla |
#23 | show_context_in_blocks-2603454-22.interdiff.txt | 1.83 KB | Arla |
Comments
Comment #2
StryKaizerComment #3
borisson_This interdiff is much larger than the actual change, I only removed the
if (count($options) > 1 || !$definition->isRequired()) {
-statement and reindented the form element definition inside the if statement. So this is actually a 1-line change.We can remove the entire if-statement because we should always show the dropdown. With the previous solution the dropdown was only shown when either
a) More than 1 condition option was available.
b) The context was not required.
So that means we would only hide the dropdown when there was 1 condition option available and the condition is required.
In this issue, we want that behaviour to change so that the condition is also shown for required conditions with only 1 option.
Comment #4
borisson_Comment #6
borisson_Attached patch fixes the
BlockLanguageTest
. TheConditionFormTest
is still failing but I can't figure out why yet. This patch should come back with 1 failing test.Comment #8
borisson_All tests should be green and the patch is much smaller now.
This makes brings back the if statement surrounding the select element to make sure it doesn't show when no options are selected (because then it's an empty select anyway).
Comment #9
Nick_vhI think this issue could greatly benefit from some screenshots why this is necessary and what impact it has on any existing core features. As I understand it, the context system isn't widely used yet, so the impact is minimal.
Comment #10
borisson_Comment #11
dsnopek+1 to this issue!
We had similar functionality briefly in #2377757: Expose Block Context mapping in the UI but it was removed before commit. I personally think it made it much easier to understand where block content is coming from.
Comment #12
Fabianx CreditAttribution: Fabianx at Tag1 Consulting commentedComment #13
borisson_Comment #14
tim.plunkettComment #15
borisson_Comment #16
MustangGB CreditAttribution: MustangGB commentedSo the select list appears if there is only one option to "choose" from, in which case shouldn't it be informational only, i.e. text and not a select list?
Comment #17
borisson_@MustangGB, that was the solution in the original patch.
I don't understand why you would do this, this would be the only place I know in core that replaces a select list with an informational text.
Comment #18
Bojhan CreditAttribution: Bojhan as a volunteer commentedThis is quite a challenging bit. We don't want to trade the confusion of not knowing what context is being used, for a select list that doesn't actually provide you with options.
Although we don't have a fully established standard yet. I would like to pursue the road of showing the text, when there is only 1 option. This should be a useful standard across core and setting here should be fine.
Comment #19
ArlaI agree on the main motivation, that it is confusing to not see what context is being used. It is certainly easily misinterpreted as "there's no context".
Replacing a select with text is also a bit confusing IMHO.
How about leaving the select element, but setting
#disabled
and perhaps adding a note (in#description
?) like "This field cannot be changed because there is only one context item available and it is required."Comment #20
ArlaHere's an implementation and screenshot of the suggestion in #19.
Comment #22
ArlaAttempting to fix fails introduced by #2339151: Conditions / context system does not allow for multiple configurable contexts, eg. language types. Seems like that issue has made the statement "the context system is not in use yet by any core feature" false. Updating IS.
Shouldn't we make the mapping visible also in the case where there's only the "None" option? That is, optional context definition with no matching contexts.
Comment #23
ArlaComment #24
dsnopekThis issue should have allowed for optional contexts: #2484645: Assigning context mapping: allow empty selection for optional contexts
Is that broken again?
Comment #25
ArlaSorry, what I mean is "shouldn't we make the mapping disabled". What we get with the patches in this issue is a select element with only the "None" option, and it is not #disabled so it has the same problem as mentioned in #16.
Comment #26
xjmComment #28
andypostComment #32
tim.plunkettWhen there are no options, it still shows an empty select list if the context is optional.
This was very confusing to more than a couple people in IRC that I talked to this week.
Why not make the condition
Comment #33
benjy CreditAttribution: benjy at Unearthed commentedYeah that definitely makes the most sense to me. +1
Comment #34
ArlaHere's that.