Remove "Highlighted" region from Bartik

I like "Featured" because it's styled differently, and makes for kind of a dramatic "call-out" effect. I've seen it used pretty frequently on Bartik sites out there "in the wild."

Highlighted, at first glance, seems like it could just be blocks ordered above the "Main page content" block in the "Content" region, other than the page title is smucked at the top. I haven't looked at / studied the CSS at all though to understand the reasoning behind it.

I am about as new a user as exists in Drupal and in web development in general. Please keep Featured! It is needed for amateur users like me for special announcements, not all the time, but on occasion. I would reduce the font size in Featured in core (it is really too big right now, and I've reduced it in my sub-theme).

Highlighted could definitely be taken out, in my opinion, and would eliminate confusion, as these comments suggest. If someone wanted something there, they could just create a block for it, and if someone is wanting the special effect similar to the CSS for Highlighted, they would be better off using Featured instead to get it to spread all the way across the page. I've often confused Highlighted and Featured when trying to alter the CSS, just because the names are so similar. Someone who wants many different regions is probably going toward Panels rather than Blocks, so I think taking out Highlighted would be a positive step toward simplification for Blocks users.

The only reason I included "highlighted" is because it was a new region for Drupal 7 core that seemed to be required for all themes. It's a region in Stark. I assumed that contrib modules might need or expect this region, and not having it in Bartik could mess things up when using such a module. I also assumed that not including the 'highlighted' region would have blocked Bartik from being added to core. So for all those reasons I put it in.

It's not really part of the design of Bartik. I agree, it's not needed for content placement. I expected (wrongly) that people would know to ignore it and not put any content in it (because magically everyone knows the full backstory of the evolution of core and never gets tripped up on Drupal WTFs).

I'd be fine with highlighted being removed. Like I said, it wasn't really part of the design in my head. If it's not a core requirement (or if that requirement has worn off), then it can go. I do agree, "featured" should stay. It's a big part of the design.

Patch please! :)

I think it also need to be removed from the template?

It needs to be removed from the template, any CSS that is associated (if any, I can't remember) and IMO we might consider the need for a BC layer for D7 because we have no idea how we're are going to cripple sites that use this region for unknown amounts of blocks, JS hooks and the like.

Removed highlighted from template also.

I wrote this one to completely wrong issue queue. Sorry about it.

Still sorry about the last one.

So, on this patch also highlighted related css has been removed from style.css file.

So are we only removing the Highlighted region? What about all of these:

  triptych_first: 'Triptych first'
  triptych_middle: 'Triptych middle'
  triptych_last: 'Triptych last'
  footer_firstcolumn: 'Footer first column'
  footer_secondcolumn: 'Footer second column'
  footer_thirdcolumn: 'Footer third column'
  footer_fourthcolumn: 'Footer fourth column'

  triptych_first: 'Triptych first'
  triptych_middle: 'Triptych middle'
  triptych_last: 'Triptych last'
  footer_firstcolumn: 'Footer first column'
  footer_secondcolumn: 'Footer second column'
  footer_thirdcolumn: 'Footer third column'
  footer_fourthcolumn: 'Footer fourth column'

These provide different layouts out of the box for users. Otherwise everything would just be stacked on top of each other.
This has been raised before here https://www.drupal.org/node/1164784#comment-5234882 and in comment 13.
But no one agreed anything there or raised them in this issue either.

If we are going to consider regions that will affect the design out the box, it will be a big change that needs more discussion and planning.
We should focus just on 'Highlighted' for now as many people and Bartik maintainers have agreed to this change in previous comments and we can raise a further issue to refactor the regions in Bartik.

So is this RTBC?

The patch applies cleanly. It removes the Highlighted region from bartik.info.yml, style.css and page.html.twig.

The highlighted region no longer displays on the Block layout page and there are no reported errors from Drupal.

And just to check the frontend, Bartik does not visually appear broken.

Setting this to RTBC.

All right. If I'm not mistaking, it needs a Drupal 8 beta phase evaluation template. Nice work DickJohnson with the patching!

RTBC++

As a personal opionion: we should finish #2375673: Split Bartik's CSS into SMACSS style components before commiting this. Rerolling this is much easier than rerolling it.

I'll postpone this to not cause problems on the CSS split issue.

This need to follow what is happening over at #1164784: “Triptych” term is not widely understood; add "Featured top" and "Featured bottom". We need to add a change record for this.
But keeping this postponed as our main concern is not having a whole ton of work to need to be done on #2375673: Split Bartik's CSS into SMACSS style components if other things get committed first.

Also tagging as "D8 upgrade path" because, like that issue, this change would result in disappearing blocks and invalid configuration without one.

Oh, additionally, let's fill out the "Unfrozen changes" and "Prioritized changes" rows of the beta evaluation, because they're both especially important for normal tasks. Edit: Like that issue, this one is borderline because of the impact on existing sites and config (which needs further discussion generally), but it's not as clearly related to usability as the other. :)

Rerolled after css -> smacss split.

I have updated the issue summary as per #32.

We now need a new code review, new screenshots as we have a new patch in #33 as all Bartik CSS files were split here #2375673: Split Bartik's CSS into SMACSS style components.

Also we need a change record like what has been created for #1164784: “Triptych” term is not widely understood; add "Featured top" and "Featured bottom" here.

When I grepped 'highlighted' these result came up:

core/themes/bartik/bartik.info.yml:  highlighted: Highlighted
core/themes/bartik/bartik.libraries.yml:      css/components/highlighted.css: {}
core/themes/bartik/css/components/highlighted.css:#highlighted {
core/themes/bartik/templates/page.html.twig: * - page.highlighted: Items for the highlighted content region.
core/themes/bartik/templates/page.html.twig:      {% if page.highlighted %}<div id="highlighted">{{ page.highlighted }}</div>{% endif %}
core/themes/seven/templates/page.html.twig: * - page.highlighted: Items for the highlighted content region.

but the patch doesn't touch page.html.twig .

Screenshot and change records coming.

Ok. New patch around with changes from #36.

Status change for testbot to test and draft Change Record: https://www.drupal.org/node/2398539

Seems like I incorrectly deleted (overwrited) the form.. sorry! Adding patch from #37 again

Grepped 'highlighted' in bartik and found nothing.

These are the screenshots tested with simplytest.me

Before

After




no more choices for highlighted region

But there's also an issue (I think outside of the scope of this issue) that when I shrink my browser to min. width it has blank space on the right side.

That's my review so I think it could be RTBC but I'll leave that to more experienced contributors.

Thanks for the screenshots! Looks good to go. Change record is also there so setting to RTBC.

The patch no longer applies

Rerolled.

@DickJohnson Can you set the issue to Needs Review to fire testbot and update tags once you have rerolled in future, thanks.

I'm going to try and retest one more time. It looks like nothing to do with the patch is breaking... I think...

Finally the test passed :) Re-upping the RTBC!!

Featured vs highlighted - I wonder how translators deal with that. Nice change. Committed 3fdbd16 and pushed to 8.0.x. Thanks!