Revert broken flexbox after Branding component creation

@Chi I'm not sure if you intended to link to a different related issue but this is currently linking to itself.

Right. Added the correct one. Though I suspect those styles had become redundant before they were moved to SDC.

I agree, so it's removed now. But i'm thinking globally about this - we are using BEM methodology, and BEM says that block-level element shouldn't have positioning styles. Normally speaking - branding component doesn't know where it will be displayed. In header, or in footer, or somewhere else. So normally except of removing `flex` property i also have to remove

  .branding {
    margin: 2.5rem 0;
  }

And add this margin or for child elements of `.branding` bem-block, or from some parent component, like `.header .branding { margin: 2.5rem 0; }`

You are right. A component should not define external geometry.

But what about other CSS rules? Here is the content of that file.

/**
 * @file
 * This file is used to style the branding block.
 */

.branding {
  flex: 0 1 40%;
}

@media screen and (min-width: 48em) {
  .branding {
    flex: 0 1 220px;
    margin: 2.5rem 0;
    text-align: left;
  }
}

.branding__site-logo {
  display: inline-block;
  width: 100%;
  max-width: 205px;
  background-color: inherit;
}

.branding__site-logo:hover,
.branding__site-logo:focus {
  background-color: inherit;
}

.branding__site-logo svg {
  width: 100%;
  max-width: 205px;
  height: auto;
}

And the corresponding HTML.

<div class="branding">
  <a href="/umami/web/en" rel="home" class="branding__site-logo">
    <img src="/umami/web/core/profiles/demo_umami/themes/umami/logo.svg" alt="Home">
  </a>
</div>

Could you find a single line here that still makes sense, besides the margin?

Note that the component has no any text nodes. And the logo is rendered via img tag.

I have removed some unused styles, attached interdiff for same.

.branding__site-logo:hover,
.branding__site-logo:focus {
  background-color: inherit;
}

This code is required, If we eliminate the background, the background color appears on the anchor tag. This will also modify the background color of the logo image.

a:hover, a:focus {
    color: #cc2a00;
    background-color: #e6eee0;
}

Hi,

Tested the patch 3379522-7.patch from comment #7, and found unused styles in Umami branding component are removed successfully from the file without any UI impact on design.

Also tested the this code, it will also modify the background color of the logo image.

a:hover, a:focus {
    color: #cc2a00;
    background-color: #e6eee0;
}

Tested steps:
1. Drupal 11 setup with Umami theme.
2. applied the patch and verified the patch changes.

Looks Good for RTBC+

Per testing #8

Still think that the whole branding.css can be dropped.

I think this is unused at the moment but this was added for a reason; to reduce the size of the logo on mobile. Looks like 🐛 Revert broken flexbox after Branding component creation Fixed caused a regression to the mobile styles. Maybe we should repurpose this issue to address that regression? It's also a good opportunity for us to figure out how we want to handle layouts with components.

Looks like #3379522: Unused styles in Umami branding component caused a regression to the mobile styles.

You reference this issue. I guess it supposed to be some other issue.

FIxed position regression.

Tested on mobile and see that the logo size changes

before

after

@smustgrave Yes! This is exactly what was fixed. It was broken while branding component was created and now fixed.

You can see styles on place now.
https://gyazo.com/2b9fea383718076ba29d7db871b9b9ba

Issue summary and title should be updated if this is fixing a bug vs removing unused code please

Think this is ready for RTBC bucket.

Committed 8e56fd3 and pushed to 11.x. Also cherry-picked to 10.1.x. Thanks!

lauriii → closed merge request !4694

Automatically closed - issue fixed for 2 weeks with no activity.