Problem/Motivation
Note: #1967460: Display the block title and populate the machine name for views blocks tries to provide an actionable first step for this issue.
From #1938062: Convert the recent_comments block to a view. Right now, Views goes out of its way to override the block form and prevent the user from creating a Views block instance title different from the View display title. This results in this UX fail:
It basically prevents the UX improvement of #1875260: Make the block title required and allow it to be hidden from taking effect, regresses from D7 in that you can no longer set a block instance title different from the View title, and confuses users by being different from every other block.
Filing as major because this is considered a blocker for all Views block conversions.
Proposed resolution
Allow users to override the View title on the block configuration form.
Remaining tasks
First, see #1967460: Display the block title and populate the machine name for views blocks.
-
We need to find a happy medium between letting content authors configure their block instances, and communicating to users that Views controls the block title dynamically. @yoroy proposed this interaction to resolve the issue:
http://dl.dropbox.com/u/538835/block-title-overrides.m4v -
There is also a concern that site builders will no longer be able to control the titles for these blocks once the block is allowed to override it. In #3 (and related issue #1957346: Add some settings on the block display to allow overrides on the block instance configuration), @dawehner suggested that we might want to allow Views admins to restrict when these settings can be overridden on a per-block level.
-
Finally, we need to find a way to describe what the view title is for the user, since Views sets it dynamically based on conditions like whether the view has no results or what arguments are supplied to it. #1957214: Title setting in views UI does not indicate when the title might be overridden also is exploring this issue within the Views UI itself.
Comment | File | Size | Author |
---|---|---|---|
#87 | Screen Shot 2013-12-05 at 7.02.05 PM.png | 70.19 KB | webchick |
#87 | Screen Shot 2013-12-05 at 6.55.39 PM.png | 59.35 KB | webchick |
#87 | Screen Shot 2013-12-05 at 6.45.13 PM.png | 57.1 KB | webchick |
#87 | Screen Shot 2013-12-05 at 6.43.59 PM.png | 56.57 KB | webchick |
#87 | Screen Shot 2013-12-05 at 6.42.37 PM.png | 59.28 KB | webchick |
Comments
Comment #1
xjmComment #3
dawehnerCouldn't this also fail in the other way round? I guess if we do it here, we should also make it clear in the views UI, that the title is set JUST in the block configuration.
It's kind of sad, that once you have the same view in multiple instances (which certainly happens once you have something like a layout builder) you would have to update each block instance in order to update just the title of the view.
Comment #4
xjmWell, this is also true of menus, or custom blocks, or any other block type, and it's actually by design. Those instances should be able to have different titles. I agree that we probably want to make it clear in the Views UI how the title would be used. My one concern at the moment is titles with argument handling, but core also doesn't provide a way to get the context for those arguments into the block yet. =/ But @webchick is adamant that block administration is a content author task and people who shouldn't have access to the Views UI should be able to change it.
@davereid suggested last night allowing tokens for the block instance title that would refer to the Views title. We could alternately expose a checkbox or something to use the display title (so long as we actually show what it is on the form). We also need to automatically generate the machine name if we do that.
Comment #5
xjmSo, it sort of confused me that this test and its friends are in the Block module, rather than the Views module, since Views provides the plugins. Anyway. Halfway through adding test coverage for the attached (which just makes Views blocks act like other blocks, except makes the view title a fallback before the display label), I decided we could add a nice toggle, like:
With the other option being:
Comment #7
xjmSomething like this.
Comment #8
xjmCouple things:
use_view_title
; it should be the title of the aforementioned container element anduse_view_title
should get an invisible title.#markup
to print the title on the page instead of in the dropdown, but it doesn't seem to support#states
, so I just put it inside the select box rather than trying to write more fancy JavaScript. ;) This might be better anyway.Comment #10
xjmI asked @yoroy and he pointed out that this is a confusing use of the select box, because the value of the title from the view is in it to begin with, and then moves out of it into the text field when you change the value. Going to propose a couple other patterns with some screenshots.
Comment #11
xjm@yoroy suggested something like this:
This takes away the end users' (possibly confusing) ability to choose between overriding the view title or not, and makes it so the view is what determines whether the title is always overridden or not. I think this might address @dawhener's concern about being update all block instance titles, because then it's sitll in the Views' admin's power to force all blocks to inherit a particular title.
Comment #12
xjmOr this:
http://dl.dropbox.com/u/538835/block-title-overrides.m4v
Comment #13
yoroy CreditAttribution: yoroy commentedMaybe something like this: http://dl.dropbox.com/u/538835/block-title-overrides.m4v
Ah crosspost
Comment #14
yoroy CreditAttribution: yoroy commentedoops
Comment #15
xjmRelated: #1957214: Title setting in views UI does not indicate when the title might be overridden
Comment #15.0
xjmUpdated issue summary.
Comment #16
xjmI updated the remaining tasks with the questions we need to address here.
Comment #16.0
xjmUpdated issue summary.
Comment #17
xjmAlright. Since this turns out to be pretty complicated architecturally, trades one UX regression for another by conflating the Views vs. Block administrative domains, has garnered a lot of resistance from my co-maintainers, and does not have consensus on anything, I've used @yoroy's middle example from #13 as the basis for #1967460: Display the block title and populate the machine name for views blocks. I made that the major bug since it addresses the 2-3 worst UX problems with the current Views block plugin, and I'm recategorizing this as a feature request.
Comment #17.0
xjmUpdated issue summary.
Comment #17.1
xjmUpdated issue summary.
Comment #18
xjmOr just #1968296: Block title != view title.
Comment #19
xjmSorry for dropping the ball on this; I had a really overwhelming couple of weeks.
I discussed this problem space at length with webchick for maintainer feedback. We did testing and there is a feature regression from Drupal 7. In Drupal 7, users can set the block title to any old string in the Blocks UI, obliterating any Views title handling whatsoever. @webchick considers both #1968296: Block title != view title and #1967460: Display the block title and populate the machine name for views blocks wontfix.
So, we need to go back to the original approach in this issue: rip out Views' override for the title. Then we're no worse off than in D7; the only difference is that multiple block instances with multiple different overridden titles are now possible in core. Then, we should improve the UI to make it clearer to users that they're overriding a dynamic title. We can incorporate some of @yoroy's ideas, but we should punt for now on trying to tell the user what the View title actually is.
I'll try to follow up more in the next few days.
Comment #20
damiankloip CreditAttribution: damiankloip commentedDo we mean removing where the title is overridden in ViewsBlock::blockBuild()?
Comment #21
tim.plunkettThat would force you to respecify the title for each block placement, and would be a regression from being able to use tokens.
But if we decide to ignore the tokens part, before we remove the ability to set a title from the Views UI, why not relabel the setting to say "Default title", with a description saying that it will just pre-fill the block label for you?
Comment #22
xjmNo, that's not the idea at all. Basically, we just change the label of the "Display block title" checkbox to something like "Override view title" or something along those lines. Views still does its full normal title handling, and the title is only replaced if the box is checked. And we uncheck it by default.
I'll roll a path to illustrate.
Comment #23
xjmThis is the general idea.
We can work on the form text/UX if we want, but functionally, this is a clean, simple compromise.
It will fail tests. I just want to get the proof of concept up before I go celebrate the gradual loss of my youth. ;)
Comment #25
dawehnerI like the simplicity of the approach.
Should be camel-cazed :)
I'm not sure but do we need to implement the settings() method to register this configuration?
There is just no need for this ternary :)
I'm wondering whether it might be helpful to tell people without administer views that they maybe need that permission in order to properly change the title of a view?
Comment #26
xjmThanks @dawehner.
Rerolled after #1927608: Remove the tight coupling between Block Plugins and Block Entities (no interdiff because I rerolled by hand). I cleaned up everything around the handling of the
override_title
setting -- it does want to be insettings()
I think, and it doesn't need its own special member property. Also fixed the ternary to check what it was actually supposed to check. ;)My gut feeling is that we shouldn't confuse the user with information about permissions they don't have, but let's get feedback from the usability team on that and on the UI pattern and text.
Edit: that screenshot is now incorrect. Uploading a new one (see below).
It's a bit wordy; I wonder if we can simplify the text without confusing people? What happens is:
Comment #27
xjmComment #28
Bojhan CreditAttribution: Bojhan commentedI am reviewing this, but will need a bit more time to come up with some ideas. I will get that up a bit later.
Comment #29
xjmThanks @Bojhan!
Tagging for tests just so I don't forget.
Comment #30
Bojhan CreditAttribution: Bojhan commentedI was intrigued to find #1968296: Block title != view title and #1967460: Display the block title and populate the machine name for views blocks closed. I don't really understand why they were won't fixed.
Upon reviewing this I was quite confused about its intend. I would assume whatever is in my "title" field is going to be the title of my block. Because users will be tremendously focused on that particular field in the task flow of changing ones title. However we currently have no signaling in the actual field that its being replaced by views, nor that its custom. This signalling is lost, and you need read the checkbox below carefully to understand its being overridden and you can override it by changing the title and pressing the checkbox.
I'd rather much prefer us to do:
For a couple reasons:
I know in the other issue we out ruled using tokens to do this, but I think that inventing a new pattern of using a checkbox to override the field that is showing above, is quite confusing. Primarily because people are really focused on the "title" field, and probably will miss the checkbox and upon seeing the checkbox have to learn the concept that changing the field, does not actually have it show up - you also need to check this checkbox to make it work.
I hope we can revisit the decision , and use tokens for this - because its a more consistent pattern, that has been validated and does not introduce this magic of changing a field, and having to click a checkbox for that change to take into effect.
Comment #31
Bojhan CreditAttribution: Bojhan commentedComment #32
xjmThe problem with #30 is that it now behaves differently from all other blocks, and it no longer provides the administrative label for the block, nor the machine name. We're back to essentially the same problem as in the screenshot in the summary. The user has to set a machine name by hand, and additionally there's no indication what the title of the block is in the admin interfaces. When I fill out this form, I have no idea what this block is going to be called in the blocks UI.
Comment #33
xjm@dawehner suggested providing a default of
viewname_displayname_count
for the machine name when the faux-token is used for the title, but that still leaves us with two problems.My [view:title] flowers
, or even other tokens. I think that leapfrogs us up into another whole realm of complexity.Comment #34
xjmMmmh, the more I think about it, the more I think the fake token will confuse people. :(
Comment #35
xjmI'll try rolling a patch so we can see what this will actually be like.
I hate this issue... We're on approach #7. :P
Comment #36
Bojhan CreditAttribution: Bojhan commented@xjm But don't we have other fields where we don't allow all replacement patterns. I definitely don't want us to go down the path of "fake tokens", but it should be clear there is only one token allowed here (which is why I didn't add the whole replacements UI). I am not sure how that will confuse people? Because they can't add their own constructs? I do agree that we can't have a token as label for a block in the block listing.
The administrative label of the block, should be added upon creation, given that this is created by views, it should inherit the title created there. Lets chat on IRC about this.
Comment #37
saltednutxjm patiently gave me a deep overview of this problem and we resolved that there is an initial step that can be taken before solving this UI problem for views blocks: #1998582: Auto-generate machine_name for Views Blocks using [viewname]_[displayname] rather than forcing manual entry
Comment #38
merlinofchaos CreditAttribution: merlinofchaos commentedIn Drupal 7 Panels world, aka content_types, the way this works is that on most panes, you get a checkbox:
If that is checked, than javascript makes the actual title appear. If that is not checked, then you automatically get whatever title that block wanted and the user doesn't have to think about it.
With Views, this is complicated one more step because the presence of that checkbox is actually a setting in the view itself; after speaking with EclipseGc, that setting is probably defaulted wrong, because it defaults to not allowing you to change the title when you place the pane. In general, that's the one setting that should be more permissive.
In D8, this appears to be complicated by deriving a machine name from the title. However, in the case of Views, it looks like #37 addresses this. I think that's a pretty good pattern: Pre-created blocks of this nature should be able to generate their own machine name. Changing the title should, IMO, be a checkbox that you have to activate.
Once you activate the checkbox, then Bojhan's mockup looks pretty good -- you enter the title, and you have a token to get the original title. Though I disagree with Bojhan; I think we should just allow all tokens relevant because people do crazy stuff with titles and if you're going to override a title that was given by the View, there's a good chance you want to do something crazy. Allowing tokens doesn't hurt. That said, at a minimum we still have to have a token for the original title, because it's common for people to just want to put a prefix/suffix around the old title.
I disagree that you should have to use a token as a matter of course. If a user is placing a block, and is required to put in a special token to get the correct title to the block that the block expects to use, then users will be confused and frustrated when they are working quickly and fail to see that they had to do this; and that they have to type a lot of words to get what should be default behavior.
Finally, in the real world the administrative title and the visible title of a piece of content diverge a lot. It should be easy and convenient for users to handle this diversion. The administrative title should mostly be provided by the view, and potentially have a checkbox or something to override it as well. I'm not sure about this last part, as blocks probably don't have the level of customization that Panels do today, so it may be much more rare that you'll need to use the administrative title to differentiate several very similar blocks. But in the real world, that is often necessary.
Comment #39
sdboyer CreditAttribution: sdboyer commentedvery relevant to this: #2025649: Add title-related methods to BlockPluginInterface to help clarify and resolve many issues. if we can get that in, Views can be a lot less backflippy here.
Comment #40
Berdir#26: views-block-title-1957276-26.patch queued for re-testing.
Comment #41.0
(not verified) CreditAttribution: commentedUpdated issue summary.
Comment #42
dawehnerThis adds a really simple checkbox based overriding, see screenshots. The label on the block listing is not overridden by that title.
Comment #43
tim.plunkettThis should be in the Block annotation class instead.
That's some weird indenting
Extra {
These should just be
$this->configuration['views_label']
Why isn't this just in the block plugin?
Comment #44
dawehnerI don't see why? We are doing that in other places, too and it just does exactly why you say as replacement.
Comment #45
tim.plunkettgrep for "this->setConfigurationValue". It appears nowhere.
Comment #47
dawehnerComment #48
tim.plunkettYes, those are all external. But every single blockSubmit() method uses $this->configuration.
Comment #49
dawehnerI should better not comment that at all.
Comment #50
dawehnerAdding tags.
Comment #51
slashrsm CreditAttribution: slashrsm commentedThis comment needs to be fixed a little bit.
Needs to be changed to:
I created a block view and positioned it to the sidebar. I tried to override the title and I can confirm that it does not save.
We probably need test coverage for that also.
Comment #52
slashrsm CreditAttribution: slashrsm commentedComment #53
dawehnerThank you for the review!
Comment #54
slashrsm CreditAttribution: slashrsm commentedNow it saves correctly, but it does not have any effect (title stays default even if I override it).
This part would probably also need test coverage.
Comment #55
dawehnerOkay okay ... more test coverage needed
Comment #56
tim.plunkettIs there any reason not to use an if/else here, and keep the $definition calls in the else? Or is that a micro-optimization?
The rest of this method is
Should the parent call be +='d into the blockSettings? Otherwise just
return parent::defaultConfiguration();
seems cleaner.Comment #57
dawehnerGood points!
Comment #59
dawehner57: vdc-1957276.patch queued for re-testing.
Comment #60
tim.plunkettMissing an 'admin_label' in here. If this doesn't fail we need tests.
Comment #62
dawehnerHa.
Comment #63
tim.plunkettThis would be better with #2025649: Add title-related methods to BlockPluginInterface to help clarify and resolve many issues, but that can happen once that's in.
RTBC if green.
Comment #64
catchI think we could improve on 'you better'. 'it is recommended to', 'it is more appropriate to'?
Why are we offering a UI option then flashing a warning on it straight away? Or to put it a different way, what's the actual use case for setting the title here - that should be explained in the description if anything.
Comment #65
webchickAFAIK this is the only thing that blocks views block conversions patches, so tagging it as a target for alpha 7.
Comment #66
dawehnerPeople argue that there are people which has access to configure blocks/place blocks but not allow to change the actual views. So we have to copy UI from views to the block configuration.
Here is a suggestion: add that functionality so we can start to do the block conversions, but maybe improve the text/UI in a follow up.
Comment #67
slashrsm CreditAttribution: slashrsm commentedI agree with this suggestion. We should probably involve some UX people to that follow-up.
Comment #68
Bojhan CreditAttribution: Bojhan commentedI am not too sure about this solution, why don't we just give a description and be done with it? I feel like we are special casing this, and it is not a pattern we can or should use elsewhere.
Drupal should inform people what their choice means through descriptions, however we shouldn't belittle them by warning them about their choice.
The text is really not up to standard of core. It uses a belittling tone "you better" and uses very technical lingo "dynamic title support". I suggest saying something like "Changing the title here, overrides the title created dynamically by [view]".
I strongly suggest to stop the "follow-up" mentality for UX issues, the whole idea of this issue is to fix the UX. You shouldn't then put the majority of the UX in a followup.
Comment #69
Bojhan CreditAttribution: Bojhan commentedComment #70
catchI think this is plenty (without the warning styling). We can make [view] link to the views UI if the module is enabled/user has permission.
Comment #71
dawehnerHere is the suggestion implemented.
Comment #72
dawehnerHere is also a screenshot of the screen.
Comment #73
yoroy CreditAttribution: yoroy commentedThanks for screenshot. I suspect bojhan meant to suggest that [view] would be replaced with the actual views name. If that's too much, then we have to add "the":
Changing the title here overrides the title created dynamically by [the view].
I was thinking
Changing the title here will remove some flexibility. (Try setting the title [directly in the view]).
With the part between () based on the acces check and the part between [] as the link.
Comment #74
dawehnerAs said on irc:
The point of that description is to tell people why doing it here is not really right.
Comment #75
dawehnerThe reason why I want to explain things is that people with just block permissions, should better not break the site functionality by just setting a title.
There are clear fundamental disagreements with the assumption whether users want to understand things or not. @xjm Feel free to bring that home, but I cannot really support and issue assuming that people are stupid.
Comment #76
yoroy CreditAttribution: yoroy commentedPeople aren't stupid, I'm just questioning how helpful it is to talk about overrides, arguments dynamic title support from arguments.
Dawehner mentioned he'd be happy with bojhan's version. How about this then:
Changing the title here means it cannot be dynamically altered anymore. (Try changing it directly in [the view].)
Comment #77
dawehnerNew version.
Comment #78
tim.plunkettThe parentheses are wrong here, the @name is passed to url(), it needs to be passed to t().
Comment #79
dawehnerGood catch.
Comment #80
dawehnerSome inspirational idea: let's link to the proper edit url.
Comment #81
tim.plunkettTruly inspired! :)
Thanks UXers and dawehner for working through this.
Comment #86
dawehnerA unit test just needs an additional injection.
Comment #87
webchickAWESOME work, folks!! I'm fine to commit this as-is, since it resolves the bug in the issue title. So I will do that tomorrow.
However! Since Daniel gets up when I go to bed, I am curious about "just one more thing." :P
"Recent content" block in HEAD:
"Content" block created via adding a block display to the Front page view:
...and with Override title checked:
The one remaining visual problem here is the machine name field does not follow the standard pattern (also, that extra fieldset around the title is something non-standard we don't do elsewhere). So, just asking (and it's fine if this is better as a follow-up), would it be possible for this instead to look like so (standard machine name pattern with text field simply disabled):
...and with Override title checked (field becomes non-disabled, description shows up):
...because then there is pretty much literally no difference between the two and so all UX concerns around Views vs. normal blocks are addressed.
Comment #88
webchickBtw, now that I think about it, that dynamic description thing is probably more trouble than it's worth. So it'd just be the checkbox toggling the disabled property (and possibly some CSS; Firebug didn't seem to change it to grey when I did that) on the text field then.
Comment #89
webchickOk, didn't catch Daniel, but spoke to Tim and he confirmed this idea was not completely crack-addled, so spun off #2151731: Views blocks' machine names do not fit the standard Drupal pattern.
AAAAAAAND!
Committed and pushed to 8.x. YEAH!! :D
Thank you SO much, all, for slogging through this. Now let's get those damn blocks converted. :D
Comment #90
webchickOops. :)
Comment #91
dawehnerThank you so much more! See you in all the other issues.