HubSpot / cms-theme-boilerplate

A straight-forward starting point for building a great website on the HubSpot CMS
https://boilerplate.hubspotcms.com
Other
355 stars 355 forks source link

Changed deprecated word-break: break-word to overflow-wrap #410

Closed jsines closed 2 years ago

jsines commented 2 years ago

Types of change

Description

Removes use of deprecated word-break: break-word and switches to overflow-wrap: break-word.

Checklist

People to notify

CC: @jasonnrosa

JacobLett commented 1 year ago

I have had to remove this property on a few sites because it was causing individual characters breaking to a new line. For example.

Finall y

Is this added just to prevent horizontal scrolling from long strings of text? I personally think this should be removed.

jasonnrosa commented 1 year ago

Hey, @JacobLett that is correct - it is added to prevent a break in responsiveness on the page in the event that an individual word overflows out of the normal boundary of the page. This is something we've had customers reach out about prior to having a solution in place which is why it has been implemented. Overall we've found that the issues of having the horizontal scroll outweigh the line wrapping from a user experience perspective as the horizontal scroll can make the page harder to navigate. This is largely here as a fallback for extreme scenarios with long words or large font sizes, but I think generally, the preferred solution is to reduce the font size on mobile to allow for words to fit better on the page.

Do you have any scenarios where removing this helped (but didn't break responsiveness/create horizontal scroll on the page)?

JacobLett commented 1 year ago

@jasonnrosa ok that makes sense then. I think if a company doesn't have developer support this is good. If this setting is turned off, it requires a developer to decrease the font sizes like you mentioned when appropriate.

Most of the time I removed this and decreased font sizes on mobile. Spot-checked them for areas that were breaking layout.

johnsfuller commented 1 year ago

I'll also chime in here... I think choosing between text break vs non text break in large font mobile scenarios is like choosing between mold or mildew for dinner – neither is desirable.

Most clients' web traffic is now 50% or more for mobile. I think it's time to add mobile text size fields.

jasonnrosa commented 1 year ago

Thank you both!

@JacobLett agreed on the preferred solution there - I think we mainly have this as a fallback just in case. E.g. boilerplate is used as a starting point for a lot of marketplace themes that might not have as much interaction with a customer as opposed to a 1:1 with a developer/agency (and even if there were more granular font options there is always a chance a user doesn't use them correctly). Do you think it is worth adding a comment in the style sheet that notes that this is some optional CSS and why it is in place, so a dev going to boilerplate for the first time can remove it if they prefer?

@johnsfuller yes agreed I think both are not great solutions (horizontal scroll or word wrap) but as mentioned above the CSS above is largely just a fallback for extreme scenarios for the preferred of the two experiences (I think seeing a piece of wrapped text is not great but I think navigating a site that scrolls horizontally is much more noticeable/a worse experience).

With that being said, fully agreed that mobile options are the best path forward here but it is something we want to think on a little bit more to get correct from a CMS perspective (e.g. determine if this should be more of a built-in feature in theme settings vs. adding additional fields) before we officially add it as a recommendation here to boilerplate.

Out of curiosity, how do you currently handle mobile options for theme settings? Do you offer a setting for each spot where font sizes can be changed in the theme right now or do you have a single setting that can reduce all font sizes on mobile X % (or something entirely different)?

JacobLett commented 1 year ago

I seem to run into this with hero banners and other sections that use headings that are bigger then the default h1. Like display text. I would then add a field to specify a mobile % of the default in that module. So it is a case by case edit.

I would be in favor of having a note in the CSS just to say this is a fallback and you may want to remove this and solve for mobile heading sizes.

I started questioning this because I wanted to know the rationale behind the setting since I kept on removing it. "That's the nature of the web baby. You can't control everything." *(single tear)

johnsfuller commented 1 year ago

We have a select option to do one of the following

  1. automatically make heading fonts smaller on mobile (works most of the time)
  2. give users individual control over heading mobile font sizes
  3. do nothing
jasonnrosa commented 1 year ago

Thank you both for the insight into how you're currently solving this - that is very helpful! I like the idea of an automatic option but also allowing users to override more granularly where they want and/or allowing for a percentage reduction on mobile. I spun up an issue here for tracking adding this to boilerplate and will add a comment per your suggestion @JacobLett to the CSS file in the interim so the purpose is clearer.