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.

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

Try more universal rich text controls #2085

Merged
merged 55 commits into from
Feb 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
e9cd340
Update image-banner.liquid
kjellr Nov 1, 2022
c4479c3
Update main-product.liquid
kjellr Nov 1, 2022
3745d9a
Update collapsible-content.liquid
kjellr Nov 1, 2022
46942e4
Update multicolumn.liquid
kjellr Nov 1, 2022
16f4d89
Update collection-list.liquid
kjellr Nov 1, 2022
ba1699b
Update featured-collection.liquid
kjellr Nov 1, 2022
de33a83
Update main-list-collections.liquid
kjellr Nov 1, 2022
584803f
Update featured-blog.liquid
kjellr Nov 1, 2022
558ebec
Update featured-product.liquid
kjellr Nov 1, 2022
1105c83
Update footer.liquid
kjellr Nov 1, 2022
0ee4f5d
Update newsletter.liquid
kjellr Nov 1, 2022
9fc69f4
Update slideshow.liquid
kjellr Nov 1, 2022
c78bbe3
Revert "Update image-banner.liquid"
kjellr Nov 1, 2022
5006a6a
Fix image banner implementation.
kjellr Nov 1, 2022
29691f1
Update collage.liquid
kjellr Nov 1, 2022
5397daa
Update collapsible_tab in Main Product Section
kjellr Nov 2, 2022
96be791
Update image-with-text.liquid
kjellr Nov 2, 2022
4f41687
Update product-recommendations.liquid
kjellr Nov 2, 2022
4d250af
Update contact-form.liquid
kjellr Nov 2, 2022
7824498
Update video.liquid
kjellr Nov 2, 2022
d4e822b
Update collapsible-content.liquid
kjellr Nov 9, 2022
a9531cb
Add a little bit of inline rte CSS.
kjellr Nov 9, 2022
a0d1474
Update rich-text heading to use inline RTE instead of normal RTE.
kjellr Nov 10, 2022
3ce78d4
Remove paragraphs.
kjellr Nov 10, 2022
36273a4
Remove paragraph tags from homepage rich text heading.
kjellr Nov 10, 2022
2dbbf2e
Move inline rte code inside of the main rte css file.
kjellr Nov 17, 2022
a4e3421
Remove unintentional whitespace changes.
kjellr Nov 17, 2022
ef14dcf
Whitespace update.
kjellr Nov 17, 2022
74b7468
Code cleanup.
kjellr Nov 17, 2022
aa068d3
Merge branch 'main' into try-rich-text-in-more-places
kjellr Nov 17, 2022
d891746
Merge branch 'main' into try-rich-text-in-more-places
ludoboludo Dec 5, 2022
c8bd24c
re add classes in icon with text snippet after rebase
ludoboludo Dec 5, 2022
d70dab2
Move link styles to base.css
kjellr Dec 6, 2022
fd39a8e
Remove unnecessary asset loading.
kjellr Dec 6, 2022
dd2942b
Use RTE styles for the slideshow subhead instead.
kjellr Dec 6, 2022
cd07536
Slideshow and image banner special cases.
kjellr Dec 6, 2022
1be9b29
Remove richtext from collapsible row.
kjellr Dec 6, 2022
09c1882
Tidying up.
kjellr Dec 6, 2022
5914938
Remove component-rte.css from collage.liquid
kjellr Dec 6, 2022
4fd4772
Load the rte styles in slideshow.liquid.
kjellr Dec 6, 2022
d6e6f33
Remove unnecessary rule.
kjellr Dec 6, 2022
1cf7b3b
Update classname to inline-richtext.
kjellr Dec 8, 2022
610e6c4
Revert back to the existing richtext field in rich-text.liquid heading.
kjellr Dec 13, 2022
2659eba
Use inline-heading class for richtext heading.
kjellr Dec 13, 2022
4a472d5
Merge branch 'main' into try-rich-text-in-more-places
kjellr Dec 13, 2022
2428e48
Merge branch 'main' into try-rich-text-in-more-places
kjellr Dec 15, 2022
5e1989a
Fix link color inconsistencies.
kjellr Dec 16, 2022
8652936
Merge branch 'main' into try-rich-text-in-more-places
kjellr Jan 11, 2023
d956d6f
Move banner--desktop-transparent styles.
kjellr Jan 11, 2023
4eae366
Merge branch 'main' into try-rich-text-in-more-places
kjellr Feb 13, 2023
37e7ca8
Combine CSS Rules.
kjellr Feb 21, 2023
4a018e9
Combine CSS rules.
kjellr Feb 21, 2023
c5535db
Update wrapper tags for Image Banner & Slideshow.
kjellr Feb 22, 2023
7d9f077
Remove duplicate calls to component-rte.css
kjellr Feb 22, 2023
00292c9
Reformat link color rules to be more readable.
kjellr Feb 22, 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
19 changes: 15 additions & 4 deletions assets/base.css
Original file line number Diff line number Diff line change
Expand Up @@ -730,19 +730,31 @@ details > * {
}

.underlined-link,
.customer a {
color: rgba(var(--color-link), var(--alpha-link));
.customer a,
.inline-richtext a {
text-underline-offset: 0.3rem;
text-decoration-thickness: 0.1rem;
transition: text-decoration-thickness ease 100ms;
}

.underlined-link,
.customer a {
color: rgba(var(--color-link), var(--alpha-link));
}

.inline-richtext a,
.rte.inline-richtext a {
color: currentColor;
}

.underlined-link:hover,
.customer a:hover {
.customer a:hover,
.inline-richtext a:hover {
color: rgb(var(--color-link));
text-decoration-thickness: 0.2rem;
}


.icon-arrow {
width: 1.5rem;
}
Expand Down Expand Up @@ -979,7 +991,6 @@ summary::-webkit-details-marker {
}

.title-wrapper-with-link a {
color: rgb(var(--color-link));
margin-top: 0;
flex-shrink: 0;
}
Expand Down
16 changes: 16 additions & 0 deletions assets/section-image-banner.css
Original file line number Diff line number Diff line change
Expand Up @@ -436,3 +436,19 @@
.banner__box > * + .banner__buttons {
margin-top: 2rem;
}

@media screen and (max-width: 749px) {
.banner:not(.slideshow) .rte a,
.banner:not(.slideshow) .inline-richtext a:hover,
.banner:not(.slideshow) .rte a:hover {
color: currentColor;
}
}
Comment on lines +440 to +446
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Does this need to be in a media query? If it's true all the way from 0 - 749px then it should be outside of a media query.

Copy link
Contributor Author

@kjellr kjellr Feb 22, 2023

Choose a reason for hiding this comment

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

It doesn't technically need to. But in this case, we have multiple rules that are inherited correctly on desktop by default.

We could definitely remove the max-width media query here, but then we'd need to take corrective action on 750 and above to replicate the way it would've normally appeared. This doubled the CSS rules, and seemed heavy-handed to me (especially considering there are lots of other (max-width: 749px) breakpoints used in this file already).

Basically, it's a choice between this:

@media screen and (max-width: 749px) {
  .banner:not(.slideshow) .rte a,
  .banner:not(.slideshow) .inline-richtext a:hover,
  .banner:not(.slideshow) .rte a:hover {
    color: currentColor;
  }
}

... or this:

.banner:not(.slideshow) .rte a,
.banner:not(.slideshow) .inline-richtext a:hover,
.banner:not(.slideshow) .rte a:hover {
  color: currentColor;
}

@media screen and (min-width: 750px) {
  .banner:not(.slideshow) .rte a,
  .banner:not(.slideshow) .rte a:hover {
    color: rgba(var(--color-link),var(--alpha-link));
  }

  .banner:not(.slideshow) .inline-richtext a:hover {
    color: var(--color-link);
  }

inherit doesn't work in this case, so you're stuck specifying the same variables that are assigned in the original rules from component-rte.css and base.css. All in all, I felt that just using the max-width rule was simpler. Happy to change if you feel strongly though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for looking into this. In general I think we should pick either min-width or max-width and stick to it but we already do this all over the CSS. I won't be a stickler here.


@media screen and (min-width: 750px) {
.banner--desktop-transparent .rte a,
.banner--desktop-transparent .inline-richtext a:hover,
.banner--desktop-transparent .rte a:hover {
color: currentColor;
}
}
4 changes: 2 additions & 2 deletions sections/collage.liquid
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

<div class="color-{{ section.settings.color_scheme }} gradient isolate">
<div class="page-width{% if section.settings.heading == blank %} no-heading{% endif %} section-{{ section.id }}-padding">
<h2 class="collage-wrapper-title {{ section.settings.heading_size }}">{{ section.settings.heading | escape }}</h2>
<h2 class="collage-wrapper-title inline-richtext {{ section.settings.heading_size }}">{{ section.settings.heading }}</h2>
<div class="collage{% if section.settings.mobile_layout == 'collage' %} collage--mobile{% endif %}">
{%- for block in section.blocks -%}
<div
Expand Down Expand Up @@ -208,7 +208,7 @@
},
"settings": [
{
"type": "text",
"type": "inline_richtext",
"id": "heading",
"default": "Multimedia collage",
"label": "t:sections.collage.settings.heading.label"
Expand Down
8 changes: 4 additions & 4 deletions sections/collapsible-content.liquid
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
<p class="caption-with-letter-spacing">{{ section.settings.caption | escape }}</p>
{% endif %}
{%- if section.settings.heading != blank -%}
<h2 class="collapsible-content__heading {{ section.settings.heading_size }}">
{{ section.settings.heading | escape }}
<h2 class="collapsible-content__heading inline-richtext {{ section.settings.heading_size }}">
{{ section.settings.heading }}
</h2>
{%- else -%}
<h2 class="visually-hidden">{{ 'accessibility.collapsible_content_title' | t }}</h2>
Expand Down Expand Up @@ -67,7 +67,7 @@
>
<summary id="Summary-{{ block.id }}-{{ section.id }}">
{% render 'icon-accordion', icon: block.settings.icon %}
<h3 class="accordion__title h4">
<h3 class="accordion__title inline-richtext h4">
{{ block.settings.heading | default: block.settings.page.title }}
</h3>
{% render 'icon-caret' %}
Expand Down Expand Up @@ -106,7 +106,7 @@
"label": "t:sections.collapsible_content.settings.caption.label"
},
{
"type": "text",
"type": "inline_richtext",
"id": "heading",
"label": "t:sections.collapsible_content.settings.heading.label",
"default": "Collapsible content"
Expand Down
6 changes: 3 additions & 3 deletions sections/collection-list.liquid
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
<div class="collection-list-wrapper page-width isolate{% if show_mobile_slider %} page-width-desktop{% endif %}{% if section.settings.title == blank %} no-heading{% endif %}{% if section.settings.show_view_all == false or section.blocks.size > collections.size %} no-mobile-link{% endif %} section-{{ section.id }}-padding">
{%- unless section.settings.title == blank -%}
<div class="title-wrapper-with-link{% if show_mobile_slider %} title-wrapper--self-padded-tablet-down{% else %} title-wrapper--self-padded-mobile{% endif %} title-wrapper--no-top-margin">
<h2 id="SectionHeading-{{ section.id }}" class="collection-list-title {{ section.settings.heading_size }}">
{{ section.settings.title | escape }}
<h2 id="SectionHeading-{{ section.id }}" class="collection-list-title inline-richtext {{ section.settings.heading_size }}">
{{ section.settings.title }}
</h2>

{%- if section.settings.show_view_all and show_mobile_slider -%}
Expand Down Expand Up @@ -127,7 +127,7 @@
},
"settings": [
{
"type": "text",
"type": "inline_richtext",
"id": "title",
"default": "Collections",
"label": "t:sections.collection-list.settings.title.label"
Expand Down
6 changes: 3 additions & 3 deletions sections/contact-form.liquid
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
<div class="color-{{ section.settings.color_scheme }} gradient">
<div class="contact page-width page-width--narrow section-{{ section.id }}-padding">
{%- if section.settings.heading != blank -%}
<h2 class="title title-wrapper--no-top-margin {{ section.settings.heading_size }}">
{{ section.settings.heading | escape }}
<h2 class="title title-wrapper--no-top-margin inline-richtext {{ section.settings.heading_size }}">
{{ section.settings.heading }}
</h2>
{%- else -%}
<h2 class="visually-hidden">{{ 'templates.contact.form.title' | t }}</h2>
Expand Down Expand Up @@ -137,7 +137,7 @@
},
"settings": [
{
"type": "text",
"type": "inline_richtext",
"id": "heading",
"default": "Contact form",
"label": "Heading"
Expand Down
6 changes: 3 additions & 3 deletions sections/featured-blog.liquid
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
<div class="page-width-desktop isolate{% if posts_displayed < 3 %} page-width-tablet{% endif %} section-{{ section.id }}-padding">
{%- unless section.settings.heading == blank -%}
<div class="title-wrapper-with-link{% if posts_displayed > 2 %} title-wrapper--self-padded-tablet-down{% else %} title-wrapper--self-padded-mobile{% endif %} title-wrapper--no-top-margin">
<h2 id="SectionHeading-{{ section.id }}" class="blog__title {{ section.settings.heading_size }}">
{{ section.settings.heading | escape }}
<h2 id="SectionHeading-{{ section.id }}" class="blog__title inline-richtext {{ section.settings.heading_size }}">
{{ section.settings.heading }}
</h2>

{%- if section.settings.blog != blank
Expand Down Expand Up @@ -142,7 +142,7 @@
},
"settings": [
{
"type": "text",
"type": "inline_richtext",
"id": "heading",
"default": "Blog posts",
"label": "t:sections.featured-blog.settings.heading.label"
Expand Down
5 changes: 3 additions & 2 deletions sections/featured-collection.liquid
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,10 @@

<div class="color-{{ section.settings.color_scheme }} isolate gradient">
<div class="collection section-{{ section.id }}-padding{% if section.settings.full_width %} collection--full-width{% endif %}">

<div class="collection__title title-wrapper title-wrapper--no-top-margin page-width{% if show_mobile_slider %} title-wrapper--self-padded-tablet-down{% endif %}{% if show_desktop_slider %} collection__title--desktop-slider{% endif %}">
{%- if section.settings.title != blank -%}
<h2 class="title {{ section.settings.heading_size }}">{{ section.settings.title | escape }}</h2>
<h2 class="title inline-richtext {{ section.settings.heading_size }}">{{ section.settings.title }}</h2>
{%- endif -%}
{%- if section.settings.description != blank
or section.settings.show_description
Expand Down Expand Up @@ -150,7 +151,7 @@
},
"settings": [
{
"type": "text",
"type": "inline_richtext",
"id": "title",
"default": "Featured collection",
"label": "t:sections.featured-collection.settings.title.label"
Expand Down
10 changes: 5 additions & 5 deletions sections/featured-product.liquid
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@
{% render block %}
{%- when 'text' -%}
<p
class="product__text{% if block.settings.text_style == 'uppercase' %} caption-with-letter-spacing{% elsif block.settings.text_style == 'subtitle' %} subtitle{% endif %}"
class="product__text inline-richtext{% if block.settings.text_style == 'uppercase' %} caption-with-letter-spacing{% elsif block.settings.text_style == 'subtitle' %} subtitle{% endif %}"
{{ block.shopify_attributes }}
>
{{- block.settings.text -}}
Expand Down Expand Up @@ -524,7 +524,7 @@
"name": "t:sections.featured-product.blocks.text.name",
"settings": [
{
"type": "text",
"type": "inline_richtext",
"id": "text",
"default": "Text block",
"label": "t:sections.featured-product.blocks.text.settings.text.label"
Expand Down Expand Up @@ -910,7 +910,7 @@
"label": "t:sections.main-product.blocks.icon_with_text.settings.image_1.label"
},
{
"type": "text",
"type": "inline_richtext",
"id": "heading_1",
"default": "Heading",
"label": "t:sections.main-product.blocks.icon_with_text.settings.heading_1.label",
Expand Down Expand Up @@ -1106,7 +1106,7 @@
"label": "t:sections.main-product.blocks.icon_with_text.settings.image_2.label"
},
{
"type": "text",
"type": "inline_richtext",
"id": "heading_2",
"default": "Heading",
"label": "t:sections.main-product.blocks.icon_with_text.settings.heading_2.label",
Expand Down Expand Up @@ -1302,7 +1302,7 @@
"label": "t:sections.main-product.blocks.icon_with_text.settings.image_3.label"
},
{
"type": "text",
"type": "inline_richtext",
"id": "heading_3",
"default": "Heading",
"label": "t:sections.main-product.blocks.icon_with_text.settings.heading_3.label",
Expand Down
12 changes: 6 additions & 6 deletions sections/footer.liquid
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
{%- endstyle -%}

<footer class="footer color-{{ section.settings.color_scheme }} gradient section-{{ section.id }}-padding">
{%- liquid
{%- liquid
assign has_social_icons = true
if settings.social_facebook_link == blank and settings.social_instagram_link == blank and settings.social_youtube_link == blank and settings.social_tiktok_link == blank and settings.social_twitter_link == blank and settings.social_pinterest_link == blank and settings.social_snapchat_link == blank and settings.social_tumblr_link == blank and settings.social_vimeo_link == blank
assign has_social_icons = false
Expand Down Expand Up @@ -68,7 +68,7 @@
{%- for block in section.blocks -%}
<div class="footer-block grid__item{% if block.type == 'link_list' %} footer-block--menu{% endif %}" {{ block.shopify_attributes }}>
{%- if block.settings.heading != blank -%}
<h2 class="footer-block__heading">{{- block.settings.heading | escape -}}</h2>
<h2 class="footer-block__heading inline-richtext">{{- block.settings.heading -}}</h2>
{%- endif -%}

{%- case block.type -%}
Expand Down Expand Up @@ -141,7 +141,7 @@
{%- if section.settings.newsletter_enable -%}
<div class="footer-block__newsletter">
{%- if section.settings.newsletter_heading != blank -%}
<h2 class="footer-block__heading">{{ section.settings.newsletter_heading | escape }}</h2>
<h2 class="footer-block__heading inline-richtext">{{ section.settings.newsletter_heading }}</h2>
{%- endif -%}
{%- form 'customer', id: 'ContactFooter', class: 'footer__newsletter newsletter-form' -%}
<input type="hidden" name="contact[tags]" value="newsletter">
Expand Down Expand Up @@ -391,7 +391,7 @@
"name": "t:sections.footer.blocks.link_list.name",
"settings": [
{
"type": "text",
"type": "inline_richtext",
"id": "heading",
"default": "Quick links",
"label": "t:sections.footer.blocks.link_list.settings.heading.label"
Expand Down Expand Up @@ -431,7 +431,7 @@
"name": "t:sections.footer.blocks.text.name",
"settings": [
{
"type": "text",
"type": "inline_richtext",
"id": "heading",
"default": "Heading",
"label": "t:sections.footer.blocks.text.settings.heading.label"
Expand Down Expand Up @@ -527,7 +527,7 @@
"label": "t:sections.footer.settings.newsletter_enable.label"
},
{
"type": "text",
"type": "inline_richtext",
"id": "newsletter_heading",
"default": "Subscribe to our emails",
"label": "t:sections.footer.settings.newsletter_heading.label"
Expand Down
12 changes: 5 additions & 7 deletions sections/image-banner.liquid
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,10 @@
{%- for block in section.blocks -%}
{%- case block.type -%}
{%- when 'heading' -%}
<h2 class="banner__heading {{ block.settings.heading_size }}" {{ block.shopify_attributes }}>
<span>{{ block.settings.heading | escape }}</span>
</h2>
<h2 class="banner__heading inline-richtext {{ block.settings.heading_size }}" {{ block.shopify_attributes }}>{{ block.settings.heading }}</h2>
{%- when 'text' -%}
<div class="banner__text {{ block.settings.text_style }}" {{ block.shopify_attributes }}>
<span>{{ block.settings.text | escape }}</span>
<div class="banner__text rte {{ block.settings.text_style }}" {{ block.shopify_attributes }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<div class="banner__text rte {{ block.settings.text_style }}" {{ block.shopify_attributes }}>
<div class="banner__text inline-richtext {{ block.settings.text_style }}" {{ block.shopify_attributes }}>

Just wondering if the rte class here should be inline-richtext because of the "text" block now being "inline_richtext"? - screenshot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! In this specific case, the text mimics rte styles intentionally for visual consistency reasons: to ensure this text shares styles with the supporting text on the Image & Text block. We considered changing this field to richtext, but avoided it for now as outlined here and in the decision log. We can always re-address in a followup PR if we change our minds. 👏

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the context, Kjell

<p>{{ block.settings.text }}</p>
</div>
{%- when 'buttons' -%}
<div
Expand Down Expand Up @@ -338,7 +336,7 @@
"limit": 1,
"settings": [
{
"type": "text",
"type": "inline_richtext",
"id": "heading",
"default": "Image banner",
"label": "t:sections.image-banner.blocks.heading.settings.heading.label"
Expand Down Expand Up @@ -371,7 +369,7 @@
"limit": 1,
"settings": [
{
"type": "text",
"type": "inline_richtext",
kjellr marked this conversation as resolved.
Show resolved Hide resolved
"id": "text",
"default": "Give customers details about the banner image(s) or content on the template.",
"label": "t:sections.image-banner.blocks.text.settings.text.label"
Expand Down
6 changes: 3 additions & 3 deletions sections/image-with-text.liquid
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@
{%- for block in section.blocks -%}
{% case block.type %}
{%- when 'heading' -%}
<h2 class="image-with-text__heading {{ block.settings.heading_size }}" {{ block.shopify_attributes }}>
{{ block.settings.heading | escape }}
<h2 class="image-with-text__heading inline-richtext {{ block.settings.heading_size }}" {{ block.shopify_attributes }}>
{{ block.settings.heading }}
</h2>
{%- when 'caption' -%}
<p
Expand Down Expand Up @@ -297,7 +297,7 @@
"limit": 1,
"settings": [
{
"type": "text",
"type": "inline_richtext",
"id": "heading",
"default": "Image with text",
"label": "t:sections.image-with-text.blocks.heading.settings.heading.label"
Expand Down
4 changes: 2 additions & 2 deletions sections/main-list-collections.liquid
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{{ 'section-collection-list.css' | asset_url | stylesheet_tag }}

<div class="page-width">
<h1 class="title title--primary">{{ section.settings.title | escape }}</h1>
<h1 class="title title--primary inline-richtext">{{ section.settings.title }}</h1>
{%- liquid
case section.settings.sort
when 'products_high', 'products_low'
Expand Down Expand Up @@ -45,7 +45,7 @@
"class": "section",
"settings": [
{
"type": "text",
"type": "inline_richtext",
"id": "title",
"label": "t:sections.main-list-collections.settings.title.label",
"default": "Collections"
Expand Down
Loading