Similar to #3115429: Create theme settings for mobile menu, we need a theme setting. This one will be to change the appearance of the site-branding block.

This issue is to create the theme setting, and then apply a CSS class to the site-branding block. This issue does not cover the styling.

The theme setting should be a select list labeled "header site branding background color", with three options: blue background (the default), gray background, and white background.

The CSS class can be the following:

  • site-branding--bg-blue
  • site-branding--bg-gray
  • site-branding--bg-white

CommentFileSizeAuthor
#11 3125483-11.patch1.78 KBkostyashupenko
#8 3125483-8.patch1.7 KBkostyashupenko
#5 branding-bg-settings.png69.7 KBUjval Shah
#5 blue-branding.png10.93 KBUjval Shah
#5 gray-branding.png7.8 KBUjval Shah
#5 white-branding.png7.95 KBUjval Shah
#3 3125483-3.patch3.32 KBkostyashupenko
olivero-site-branding-bgs.png153.87 KBmherchel

Comments

mherchel created an issue. See original summary.

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Assigned: Unassigned » kostyashupenko
kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Assigned: kostyashupenko » Unassigned
Status: Active » Needs review
StatusFileSize
new 3125483-3.patch3.32 KB
kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented

Ready for review

Ujval Shah’s picture

Ujval Shah commented
StatusFileSize
new white-branding.png7.95 KB
new gray-branding.png7.8 KB
new blue-branding.png10.93 KB
new branding-bg-settings.png69.7 KB

@kostyashupenko

  1. Tested visually and its working as expected.
  2. Background color selection with different option is available.
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Fixed

Perfect! Committed. Thanks @kostyashupenko, and thanks for the review, @Ujval Shah!

dunebl’s picture

dunebl
Primary language French
commented
Status: Fixed » Needs work

This is my first message for this project, thus I would like firstly to thank you for this so beautiful theme... this is a very nice job!!!
I have 3 comments:

1-This patch will not store the setting in the UI if the theme is a subtheme of olivero.
At several place there is:
theme_get_setting('mobile_menu_all_widths', 'olivero')
and it should be

$current_theme=\Drupal::service('theme.manager')->getActiveTheme()->getName();
theme_get_setting('mobile_menu_all_widths', $current_theme)

2-Maybe this is working as designed, but the class site-branding--bg-blue doesn't exists

3-I would suggest to use bg-blue, bg-white etc as class names to allow an easy re-use in other places (and maybe creating some other possibilities of background like bootstrap does)

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Status: Needs work » Needs review
StatusFileSize
new 3125483-8.patch1.7 KB

@DuneBL thanks for your review

1. https://git.drupalcode.org/project/drupal/-/blob/9.1.x/core/includes/the...
so i just removed 'olivero' as a second param

2. Modifier site-branding--bg-blue is useless in our case, since site-branding has blue bg and white text by default. So now class site-branding--bg-blue will never be added.

3. We don't prepare css-utilities, we build components. bg-gray and bg-white - are just modifiers of the site-branding component. So i can't agree with you

dunebl’s picture

dunebl
Primary language French
commented
Status: Needs review » Fixed

@kostyashupenko #8 looks good to me... and many thanks for the explanations

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Fixed » Reviewed & tested by the community

Not yet committed. Moving back to RTBC (reviewed and tested by the community)

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Version: » 8.x-1.0-alpha1
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 3125483-11.patch1.78 KB

Reroll

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Fixed

Looks good. Committed!

Status: Fixed » Closed (fixed)

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