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

Jump to bottom

Invalid HTML markup in off-canvas sidebars: widgets wrapped in li elements as children of a div #88

Closed
diegocanal opened this issue May 12, 2020 · 4 comments · Fixed by #84
Closed

Invalid HTML markup in off-canvas sidebars: widgets wrapped in li elements as children of a div #88

diegocanal opened this issue May 12, 2020 · 4 comments · Fixed by #84
Assignees
@JoryHogeveen
Labels
bug compatibility
Milestone
0.5.5

Comments

@diegocanal
Copy link

Describe the bug

In off-canvas sidebars the widgets are output wrapped in <li> HTML elements, but not as children of a <ul>, <ol>, or <menu> element –or the obsolete <dir> element–, which is invalid HTML markup (reference in MDN).

Steps to reproduce the behavior:

  1. Add a off-canvas sidebar.
  2. Add a couple of widgets inside the sidebar.
  3. Inspect the code in dev tools to see the <li> elements wrapping the widgets.

Expected behavior

In my opinion, the widgets should not be output wrapped in <li> elements, <div> seems to me semantically more neutral and appropriate.

Screenshots

Screenshot 2020-05-12 at 18 47 40

Specifications

  • Plugin Version: 0.5.4
  • WordPress Version:5.4.1
  • Other plugins installed: none
  • Theme: Twenty NineteenVersión: 1.5
  • Browser: Chrome version 81.0.4044.138 (Official Build) (64-bit)
  • URL of where the bug occurs: staging site not public
  • Location of hook placement:
    • header.php: directly after <body <?php body_class(); ?>
    • footer.php: directly after the site content, before <?php wp_footer(); ?>)
@JoryHogeveen
Copy link
Owner

Hello @diegocanal

You can change this through the ocs_register_sidebar_args filter to change it according to your theme.
WordPress by default uses <li> elements for widgets (no idea why...) so I'm trying not to change to much in WordPress defaults.
However, in this case I do agree that it might be better for me to overwrite this default since the sidebars will never be list items in my case I suppose.

@JoryHogeveen JoryHogeveen self-assigned this May 12, 2020
@JoryHogeveen JoryHogeveen added this to the 0.5.5 milestone May 12, 2020
JoryHogeveen added a commit that referenced this issue May 12, 2020
@JoryHogeveen
Fix default before and after widget args
c1f81ff 
Prefents incorrect <li> elements due to WordPress defaults.
See #88
@JoryHogeveen JoryHogeveen mentioned this issue May 12, 2020
Merged
6 tasks
@JoryHogeveen JoryHogeveen linked a pull request May 12, 2020 that will close this issue
v0.5.5 #84
Merged
6 tasks
@JoryHogeveen
Copy link
Owner

I've added a new commit to the dev branch (next update) that will fix this:
c1f81ff

@diegocanal
Copy link
Author

Thanks @JoryHogeveen for your superfast reaction.

I'm not a programmer so figuring out how to solve it using the filter would have taken me a while. Your new commit has saved me a lot time.

I love your plugin. Keep up with the good work!

@JoryHogeveen
Copy link
Owner

Hi @diegocanal
No problem! And thanks for your review!
Cheers, Jory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JoryHogeveen JoryHogeveen

Labels
bug compatibility
Projects
None yet
Milestone
0.5.5
Development

Successfully merging a pull request may close this issue.

v0.5.5 JoryHogeveen/off-canvas-sidebars
2 participants
@JoryHogeveen @diegocanal