search

grrr-amsterdam / cookie-consent

Cookie consent with accessible dialog, agnostic tag triggers and conditional content, script and embed hooks.
MIT License
63 stars 11 forks source link

Setting for h1 title tag #21

Closed Quent1Pr closed 3 years ago

Quent1Pr commented 3 years ago

Hello, For SEO purpose, I need to change the h1 tag on this line : https://github.com/grrr-amsterdam/cookie-consent/blob/08f3db56103d1da807d5b59ade64b3d84d59231c/src/dialog.mjs#L27

What is the best option to do that ? Should we add the heading tag as a configurable option (how to handle the scss) ? Is there any way to override this function ? Should I create a pull request ?

harmenjanssen commented 3 years ago

Hi QPierre1,

What's your goal with the heading? What would your preferred DOM look like?

I'm not sure if I'd want to allow configuring just the header tag, since it's so specific and then why not every tag in the template? Maybe a viable option would be to overwrite the complete template, and make the implementor responsible for the full HTML of the dialog, but that could potentially break a lot of selectors that expect a certain DOM node to be present.

I'm not sure yet, let's start with your desired output, and let's work from there. 🙂

Quent1Pr commented 3 years ago

Hi thank for your reactivity, I think make the full template personnalizable could be a good point.

My current problem is that we want only one h1 tag on our pages (due to SEO recommandations, it's probably an universal problem) and we prefer to set another level tag for the title of this dialog template. Having a h1 Cookies tag on all our pages causes a semantic issue.

It could be <div class="cookie-consent-dialog-title"> instead.

harmenjanssen commented 3 years ago

I think using a <div> for a heading causes a bigger semantic issue. 😉

I also don't think having a Javascript-generated cookie dialog with an H1 will hurt your SEO one bit, but I get why you would want to change it.

I would accept a PR in which users of this module can provide the full template via the config. Please make sure to make it a function, so the config can be passed in there as a parameter, since the current template uses config values as well. Also, there should be some small validation to make sure <form> and <button> elements are present in the DOM, so we can alert developers if their template is invalid.

Thanks!

harmenjanssen commented 3 years ago

Closed by #22.