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

Using "id="primary" for series overview page messes up with primary widget area #353

Closed
kuz-z-zma opened this issue Dec 19, 2021 · 8 comments

Comments

@kuz-z-zma
Copy link

kuz-z-zma commented Dec 19, 2021

Wordpress themes use id="primary" to list css rules for main widget area, which usually is displayed in sidebar.

Current template for series overview page uses it to style section, which creates 2 instances of id (and html and css rules state, that there should only be onу per page), which makes page to fail markup validation and completely messes most designs, even when using Default option for display.

Styling of the Primary Widget Area, with id="primary"
normalstyling

Addition of second section using id="primary" results in this mess:
second primary

Yes, anyone with html editing skills can dig in and rename it, but most people still will be getting this unusable mess.

Please reconsider naming of the ids and classes.

@stevejburge
Copy link
Member

Thanks for reporting this @kuz-z-zma

This is a legacy issue that may be a breaking change for some sites, but we'll take a look at it

@htmgarcia
Copy link
Contributor

@olatechpro
Copy link
Collaborator

Sincerely speaking, i won't have thought of themes using primary as sidebar ID. Default theme behaviour is wrapping page content inside id="primary", id="content" and sidebar in id="sidebar", id="primary-sidebar" etc. So, we're not totally wrong wrapping the page content inside id="primary" but we can do two things about this which is probably removing id attribute completely or otherwise, themes that use id="primary" for sidebar can simply copy the template to their theme and update accordingly.
If there are themes that use our template the way it is, they may have styling issue as well if we remove the wrapper styles.

So, it's left for us to determine what may be best here as dealing with frontend development is somehow critical than backend due to different theme with different behaviour and the best we can do is sticking to what we believe to be default.

For example, TOC template structure came from WordPress TwentyEleven theme which wrapped content inside id="primary"(saw it in one of the addon template)

@htmgarcia

@olatechpro
Copy link
Collaborator

olatechpro commented Dec 22, 2021

Update: If you check 'twentytwentyone' WordPress theme header.php, content are wrapped inside primary. @htmgarcia

So, i think that's the best option we have unless we decided to remove ID attribute completely and give it a classname to enable people style the content.

Giving it random ID doesn't serve any value unless we style it in our plugin frontend css

@kuz-z-zma
Copy link
Author

Personally, I believe using those broad names like "primary" by a plugin is just asking for trouble and can be easily avoided by using something like publish-series-primary for id naming. Also using those prefixes for all styling classes/ids make it easier to find and edit css files later on by users.

@htmgarcia
Copy link
Contributor

htmgarcia commented Dec 23, 2021

@olatechpro I would think using exclusive classnames or ids is the best choice, however this requires some more thinking.

@kuz-z-zma we seems to agree on using custom classnames as an option. Please allow us some time to think about it.

@htmgarcia
Copy link
Contributor

I prefixed the ids. It will be now "series-primary" and "series-content".
b589df6

@olatechpro
Copy link
Collaborator

Thanks @htmgarcia

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants