Problem/Motivation
It's common to provide styling for base elements, elements that don't have classes. This styling is used in situations where a user can enter formatted HTML text. This can often introduce conflicts with UI components, if they are using the elements that have styling applied.
Some themes get around this by prefixing the base selectors with a wrapper class a indicates they are free text, not a UI component.
Bartik has an attempt to do this:
.region-content ul,
.region-content ol {
margin: 1em 0;
padding: 0 0 0.25em 15px; /* LTR */
}
The logic of this selector is flawed because anything can go inside the content region, not just free text.
Proposed resolution
Add a class in Classy's field.html.twig that indicates the field is formatted HTML text: formatted-text
Remaining tasks
Class name consensus
Patch
User interface changes
None
API changes
None
Data model changes
None
Beta phase evaluation
| Issue category | Task because it's a handy hook for themers |
|---|---|
| Issue priority | Not critical because it's a nice to have |
| Unfrozen changes | Unfrozen because it only changes markup |
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | Screen Shot 2015-07-27 at 21.40.14.jpg | 998.76 KB | lewisnyman |
| #1 | formatted_text_class-2539860-1.patch | 920 bytes | jeni_dc |
Comments
Comment #1
jeni_dc commentedAs it turns out I was overcomplicating this issue a bit at DrupalCamp North. Classy already contains a field--text.html.twig file that is used by all formatted text fields. That file is adding a "clearfix" class to all formatted text fields, so I just added an additional 'text-formatted' class there. Now all formatted text fields will get both classes. That template does not get used for any plain text or list fields.
I've attached a patch with the new class added, that also contains comments about why that class was added and the URL back to this issue, similar to the comments for the clearfix class.
Comment #2
lewisnymanGreat! That's a nice comment. I didn't realise we were already targeting formatted text fields. I added all text fields and added some dummy styling to show it applies correctly.
Comment #3
alexpottShouldn't we also be fixing the Bartik example in the issue summary?
Comment #4
jeni_dc commentedI'm more than happy to fix the issue in Bartik as well, but since Bartik depends on Classy, wouldn't this need to be committed first?
Comment #5
alexpott@jeni_dc it can be fixed in the same patch
Comment #6
lewisnymanI discussed this with Emma. Because the ul/ol styling in Bartik is a hot mess, changing the selector in Bartik in this issue will open up a high risk of regression. See: #2509902: Remove block.css list styles and add the code to other components.
This issue is for Classy, which means anyone sub-theming classy should benefit from this. My preference is to keep it that way instead of turning it into a problematic Bartik issue, otherwise we run the risk of this never getting in before RC and then people can't benefit from this class.
I've created a followup here: #2541252: Replace the .region-content ul/ol selector with text-formatted to refactor code + fix visual bugs
Comment #7
alexpottCommitted dead5be and pushed to 8.0.x. Thanks!
Thanks for adding the beta evaluation to the issue summary.