-
Notifications
You must be signed in to change notification settings - Fork 40
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
[UX] Place breadcrumbs above the page title for new installs. #4113
[UX] Place breadcrumbs above the page title for new installs. #4113
Comments
It just occurred to me that we could add a single NEW layout template to core and incorporate that into the default install profile to address these issues. Then all existing layout templates could be left as they are. PRO: Not changing existing layout templates, only adding new options. I added this option to the original issue. |
@stpaultim if you add the "Page title" and the "Page tabs (Local tasks)" in the layout, then you get the desired outcome; and people can then reorder these blocks as they see fit. Right?: This problem originates from the fact that the ability (or rather "trick") to make the "hard-coded" title block reorder-able came at a later stage in the Backdrop dev cycle, with #1704, in 1.7 (May 15, 2017). In that issue, we've added the ability to change things, but we have not made it the default, OOTB status. Perhaps that is what we need to reconsider. In other words, I am not opposed to the idea of another region, but I think we can do better. Perhaps the right solution here would be to have the "Page title" and the "Page tabs (Local tasks)" be added to the default core layouts OOTB (still in the existing "Top" region though - not a new one). I'm sure that existing sites would have already solved this issue manually if they needed things configured that way, so this will only happen for new installations. This would solve this bit:
Having these blocks placed in the default core layouts OOTB would spare new users the additional clicks they'd have to otherwise perform, in order to add them themselves. All they'd need to do is to edit the layout (#3946 anybody 🙂), and move the blocks around, as per their desired page style. |
I am aware that this is easy to resolve for a site builder familiar with Backdrop CMS. I'm thinking here only about the OOTB experience and first impressions. Your proposed solution is roughly what I had in mind for alternative option # - 2 and may very well be the better solution. NOTE: I am experimenting with a revised default default layout template in my own distribution and I feel like it's a huge OOTB improvement. |
Oh for sure. That is the goal here 👍 |
Going to change title of this issue to focus less on one specific solution to the problem. We can achieve a better default layout in a number of different ways (see options above). Which is best solution? Specific questions:
|
An additional region for one element feels like too much to me. I nearly always use the page title combo block. Would be nice if that could be the default. (Not sure if there are caveats, though.) |
Might this be a quick win for 1.15? My PR just reorganizes the default layout on new sites to something that makes more sense to me. WAS: PROPOSED (In this PR - backdrop/backdrop#3031): |
I'm not sure this placement seems weird and unhelpful to everyone? I have a handful of sites that place the breadcrumb below the page title. Here are 4 examples (2 of the designs below were developed for WordPress): Let's get some more feedback to see if everyone feels that above the title is a more common use-case? |
@jenlampton - after our initial exchange on Gitter in which I very hastily stated that I couldn't imagine breadcrumbs below the title, I began to imagine the kinds of designs you are posting and realized how silly my statement was. The problem I'm trying to address is making Backdrop CMS easier to use "out of the box". I'm continually thinking about what a site builder or general user can achieve with Backdrop without creating a custom theme. When I open Backdrop out of the box, the breadcrumbs feel really out of place to me and without a significant redesign, I feel like they need to be moved. Your suggestion in Gitter to make this change specific to Basis would be totally acceptable to me. I just need to figure out how to do that. :-) |
I definitely believe breadcrumbs belong above tabs. I even have a task to modify Basis (read: hack core) so that layout is the default for all our new sites. |
I still think that we could easily make some very minor changes to our default settings that would make Backdrop CMS easier to use out of the box. I understand/appreciate the feedback that have been provided and would like to discuss this in a design/ux meeting to see if we can't agree on some minor changes that would move this issue forward - or decide against these changes totally and close this issue. I've added this to the Design/UX agenda for 2020/07/30. |
What Steve Krug says about breadcrumbs in his book Don't Make Me Think:
However, he's advocating for them to be all the way at the top, above the site title, and I think that might be too much, but they definitely don't belong where they are in Basis. |
I've updated this PR, the issue title, and the description in the original post. I've narrowed the focus of this PR and issue to ONLY address the positioning of breadcrumbs on the default layout AND only when using the standard install profile. This will only effect new sites. For more info, see revised description in original post. OLD: NEW: |
We discussed this in the last Design/UX meeting. @jenlampton raised a (small?) concern about the fact that this fix enables the Title Combo block instead of relying on the default title position in Backdrop Core. I understood the concern to be that there might be better systematic ways of fixing this issue. In my view, this PR is just about changing default settings and is not intended to fix problems with the way the title is rendered. I think that fixing problems with the way in which the page title is rendered would probably have to wait until Backdrop CMS 2.0 - and I don't think we want to wait that long to make this simple change. I am curious if this issue might be eligible for a bug fix release (because then we could milestone it)? It could be considered a UX change more than new feature (it simply changes default settings on new sites). |
I've merged backdrop/backdrop#3693 into 1.x for 1.21.0! A big thank you for all the collaboration and discussion on this issue! Thank you @stpaultim, @jenlampton, @oadaeh, @docwilmot, @BWPanda, @klonos, @JSitter, @cpx, and @olafgrabienski! |
I was literally reviewing this when @quicksketch merged it. I had some concerns about the tests, namely: // Create a node page.
$page_node = $this->backdropCreateNode(array(
'type' => 'page',
'title' => "Test node title",
));
$this->backdropGet('admin/structure/layouts/manage/default');
$this->clickLink(t('Remove'), 3);
// Save the layout.
$this->backdropPost(NULL, array(), t('Save layout')); The get, link and post are all related, so I think they should be separated from the existing node creation by a blank line with no blank line/comment separating them. I'm also not sure what purpose they serve, so a comment describing this would be nice. E.g.: // Create a node page.
$page_node = $this->backdropCreateNode(array(
'type' => 'page',
'title' => "Test node title",
));
// Describe what this does...
$this->backdropGet('admin/structure/layouts/manage/default');
$this->clickLink(t('Remove'), 3);
$this->backdropPost(NULL, array(), t('Save layout')); |
Thanks @BWPanda, sounds like a small change, would you be able to file a
followup PR using this same issue number?
…On Sat, Jan 1, 2022, 6:28 PM Peter Anderson ***@***.***> wrote:
I was literally reviewing this when @quicksketch
<https://github.com/quicksketch> merged it. I had some concerns about the
tests, namely:
// Create a node page.
$page_node = $this->backdropCreateNode(array(
'type' => 'page',
'title' => "Test node title",
));
$this->backdropGet('admin/structure/layouts/manage/default');
$this->clickLink(t('Remove'), 3);
// Save the layout.
$this->backdropPost(NULL, array(), t('Save layout'));
The get, link and post are all related, so I think they should be
separated from the existing node creation by a blank line with no blank
line/comment separating them. I'm also not sure what purpose they serve, so
a comment describing this would be nice. E.g.:
// Create a node page.
$page_node = $this->backdropCreateNode(array(
'type' => 'page',
'title' => "Test node title",
));
// Describe what this does...
$this->backdropGet('admin/structure/layouts/manage/default');
$this->clickLink(t('Remove'), 3);
$this->backdropPost(NULL, array(), t('Save layout'));
—
Reply to this email directly, view it on GitHub
<#4113 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAYO3QF6SEKFMPC2YHIZODUT6Z3DANCNFSM4I5WRXLA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Thanks to everyone that has helped on this! I've been looking forward to this very small, but helpful (IMHO) change for a long time. |
@stpaultim Can you please describe the purpose of this: $this->backdropGet('admin/structure/layouts/manage/default');
$this->clickLink(t('Remove'), 3);
$this->backdropPost(NULL, array(), t('Save layout')); |
@BWPanda - Actually, I'm not sure that I can. I was unable to fix this test, so I asked about it during one of our sprints and I had a team of folks helping me. I believe that @klonos, @docwilmot, and possibly @hosef (or @bugfolder) were involved. For this part of the PR, I was simply typing what the others told me to type. Maybe one of them can better explain it. |
@BWPanda - As I think about it some more, it had to do with moving the Page Title block to different positions and then testing to see if things were in the places that we expected them to be after moving the page title. Yes, I think that code was too remove the page title block from the layout and then check to see if the results were as expected. Does this make any sense? As I recall, there was a prior test that was similar to this, but we had to modify it for the new situation. |
Yes, that pretty much sums it up @stpaultim 👍🏼 @BWPanda IIRC, one or more assertions further down that code change were failing. We had 2 options: either adapt the tests with the new positions of the blocks, or remove the block that this change adds, which reverts things back to what the previous set of tests expect, and make sure that the tests still pass after that. We decided to go with the latter. |
@quicksketch New PR to add comment to test code: backdrop/backdrop#3916 |
By @BWPanda, @stpaultim & @klonos.
By @BWPanda, @stpaultim & @klonos.
PR merged into 1.x and 1.21.x. |
Revised Description - UPDATED: Aug 14,2020 by @stpaultim
The original issue suggested changing the order of title, breadcrumbs, and tabs. The only portion of this that I've noticed real support for (and the most urgent part of this in my own mind) is moving the breadcrumbs above the title. We can revisit other ideas for improving the layout in the future.
The original PR changed the core file: core/modules/layout/config/layout.layout.default.json which meant that this change would effect all news sites regardless of install profile.
Reasons for change:
The default theme in Backdrop CMS standard install profile is Basis and positioning the breadcrumbs below the Title doesn't seem practical or appealing for Basis. While there are definitely use cases for positioning the breadcrumbs where they are, I believe that the default settings should be configured to look good and be useful in the default theme. Site admins using other themes and/or creating their own themes can easily move the breadcrumbs to the position that works best for them. The goal here is to make Basis look good and work good out of the box.
The approach in the existing PR is simply to edit the install file in the Standard Install Profile. This should only effect new sites using the Standard Install Profile and not have any effect on existing sites.
We could make the same changes directly here: core/modules/layout/config/layout.layout.default.json, but in this case these changes would effect all new sites and may have an unintended effect on users who have created their own custom install profile.
PROPOSED SOLUTION:
Based upon some feedback, I've modified the PR to only effect the standard install profile. With this PR, the standard install profile will modify the layout.layout.default.json config file to do the following:
CURRENT STATUS OF ISSUE:
Looking for feedback on this new approach. Also, need review of PR to check my syntax and variables.
Below this issue is the original issue and proposed solutions, some of which is no longer relavent.
ORIGINAL - Description of the need
Backdrop out of the box starts with a page layout that puts breadcrumbs below the page title. This seems REALLY weird and unhelpful (even though it is easy to fix).
I think that we need a better place to put the default breadcrumbs block than the current location and current layouts do not have a suitable place. We can't move the default position of the block to the header, because it just doesn't work and we can't move the default position of the title, because it's hard-coded into core templates.
While it is relatively easy to solve this problem by turning off breadcrumbs or adding your own title block in another region, I think it's poor user experience to require everyone to make this change and sets a poor first impression of Backdrop CMS (out of the box) to find your breadcrumbs in such a strange position.
ORIGINAL - Proposed solution
I propose adding another region (need ideas on what to name this region) to all core layouts between the header and the default title. Adding a region will not break existing sites and provides the opportunity to move the default breadcrumb positioning above the page title where most people would expect it to be.
Alternatives that have been considered
Additional information
The text was updated successfully, but these errors were encountered: