Skip to content
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.

Sign up for GitHub

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

Add a Color Scheme setting for Menus-Header #2622

Merged
merged 7 commits into from
May 17, 2023
Merged

Add a Color Scheme setting for Menus-Header #2622

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f722833
seperated color scheme for header and menu
lougoncharenko May 10, 2023
66b6010
made changes to remove white space as suggested from eugene
lougoncharenko May 11, 2023
2f13847
made some changes requested by ken
lougoncharenko May 11, 2023
d058da9
changed Header color scheme to Color scheme per yoann request
lougoncharenko May 15, 2023
0768647
removed header_color_scheme translation and set the label back to the…
lougoncharenko May 15, 2023
bdeeb7d
Update 15 translation files
translation-platform[bot] May 16, 2023
4789057
Update 5 translation files
translation-platform[bot] May 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions locales/en.default.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1063,6 +1063,15 @@
"show_line_separator": {
"label": "Show separator line"
},
"header__1": {
"content": "Color"
},
"header_color_scheme": {
"label": "Header color scheme"
Copy link
Contributor

@kmeleta kmeleta May 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YoannJailin I don't feel strongly at all, but just wondering if we don't want to keep the original "Color scheme" label for the header's color scheme setting. I could certainly see this adding some clarity for users, but the setting is within the "Header" section as it is, so I somewhat feel like we don't need the additional qualifier.

We only have a couple other instances of multiple color schemes in one section to pull precedent from but they just use "Color scheme" to represent the section's color scheme.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes i agree we can replace "Header color scheme" by "Color scheme". I would keep the second one named "Menu color scheme". @lougoncharenko @kmeleta

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead and changed it to display "Color Scheme" for the first one and left "Menu color scheme" for the second one

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, but I'd say we could probably clean things up a bit by removing the header_color_scheme translation entry that we added altogether, and pointing our setting label back to the global string for "Color scheme" "label": "t:sections.all.colors.label", like it was before. That we're we're not duplicating anything and there will be one less new string to translate. Let me know if that doesn't make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed it back to the original translation and removed the header_color_scheme translation

},
"menu_color_scheme": {
"label": "Menu color scheme"
},
"sticky_header_type": {
"label": "Sticky header",
"options__1": {
Expand Down
14 changes: 12 additions & 2 deletions sections/header.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@
assign localization_forms = true
endif
-%}
<header class="header header--{{ section.settings.logo_position }} header--mobile-{{ section.settings.mobile_logo_position }} page-width{% if section.settings.menu_type_desktop == 'drawer' %} drawer-menu{% endif %}{% if section.settings.menu != blank %} header--has-menu{% endif %}{% if has_app_block %} header--has-app{% endif %}{% if social_links %} header--has-social{% endif %}{% if shop.customer_accounts_enabled %} header--has-account{% endif %}{% if localization_forms %} header--has-localizations{% endif %}">
<header class="header color-{{ section.settings.color_scheme }} header--{{ section.settings.logo_position }} header--mobile-{{ section.settings.mobile_logo_position }} page-width{% if section.settings.menu_type_desktop == 'drawer' %} drawer-menu{% endif %}{% if section.settings.menu != blank %} header--has-menu{% endif %}{% if has_app_block %} header--has-app{% endif %}{% if social_links %} header--has-social{% endif %}{% if shop.customer_accounts_enabled %} header--has-account{% endif %}{% if localization_forms %} header--has-localizations{% endif %}">
{%- liquid
if section.settings.menu != blank
render 'header-drawer'
Expand Down Expand Up @@ -593,10 +593,20 @@
"default": true,
"label": "t:sections.header.settings.show_line_separator.label"
},
{
"type": "header",
"content": "t:sections.header.settings.header__1.content"
},
{
"type": "color_scheme",
"id": "color_scheme",
"label": "t:sections.all.colors.label",
"label": "t:sections.header.settings.header_color_scheme.label",
"default": "background-1"
},
{
"type": "color_scheme",
"id": "menu_color_scheme",
"label": "t:sections.header.settings.menu_color_scheme.label",
"default": "background-1"
},
{
Expand Down
4 changes: 2 additions & 2 deletions snippets/header-drawer.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
{% render 'header-drawer' %}
{% endcomment %}

<header-drawer data-breakpoint="{% if section.settings.menu_type_desktop == 'drawer' %}desktop{% else %}tablet{% endif %}">
<header-drawer data-breakpoint="{% if section.settings.menu_type_desktop == 'drawer' %}desktop{% else %}tablet{% endif %}">
<details id="Details-menu-drawer-container" class="menu-drawer-container">
<summary
class="header__icon header__icon--menu header__icon--summary link focus-inset"
Expand All @@ -16,7 +16,7 @@
{% render 'icon-close' %}
</span>
</summary>
<div id="menu-drawer" class="gradient menu-drawer motion-reduce">
<div id="menu-drawer" class="gradient menu-drawer motion-reduce color-{{ section.settings.menu_color_scheme }}">
<div class="menu-drawer__inner-container">
<div class="menu-drawer__navigation-container">
<nav class="menu-drawer__navigation">
Expand Down
4 changes: 2 additions & 2 deletions snippets/header-dropdown-menu.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
{% render 'header-dropdown-menu' %}
{% endcomment %}

<nav class="header__inline-menu">
<nav class="header__inline-menu " >
<ul class="list-menu list-menu--inline" role="list">
{%- for link in section.settings.menu.links -%}
<li>
Expand All @@ -27,7 +27,7 @@
</summary>
<ul
id="HeaderMenu-MenuList-{{ forloop.index }}"
class="header__submenu list-menu list-menu--disclosure gradient caption-large motion-reduce global-settings-popup"
class="header__submenu list-menu list-menu--disclosure color-{{ section.settings.menu_color_scheme }} gradient caption-large motion-reduce global-settings-popup"
role="list"
tabindex="-1"
>
Expand Down
2 changes: 1 addition & 1 deletion snippets/header-mega-menu.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
</summary>
<div
id="MegaMenu-Content-{{ forloop.index }}"
class="mega-menu__content gradient motion-reduce global-settings-popup"
class="mega-menu__content color-{{ section.settings.menu_color_scheme }} gradient motion-reduce global-settings-popup"
tabindex="-1"
>
<ul
Expand Down