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

Allow HTML in announcement content #33

Open
mariusa opened this issue Dec 21, 2022 · 11 comments
Open

Allow HTML in announcement content #33

mariusa opened this issue Dec 21, 2022 · 11 comments
Labels
enhancement New feature or request

Comments

@mariusa
Copy link
Contributor

mariusa commented Dec 21, 2022

Please allow HTML in announcement title, eg
Latest <b>version</b> available ${name}

@ReeceM ReeceM added the enhancement New feature or request label Dec 21, 2022
@ReeceM
Copy link
Owner

ReeceM commented Dec 22, 2022

Just following up on this, it is somewhat possible when running the custom parser on data and using the built in banner.

The only thing I have noticed in testing is that the spaces around the elements get lost. So the string would have to be Latest&nbsp;<b>version</b>&nbsp;available {% name %}. Note the addition of the &nbsp; character.

But will look at this, although I will make sure people know that they will need to sanitize their content before doing that if they aren't sure where it's from

@mariusa
Copy link
Contributor Author

mariusa commented Dec 22, 2022

Right, it would be the dev's responsiblity to sanitize content.

I'm also proposing to consider having content besides or instead of title & link.
content would just be a HTML block. This would make the announcement usable for more use cases. Now it's only a link. It could be just a simple text, HTML text with icons, even a prompt to enter email to subscribe to newsletter. Let the dev put whatever they want. If they want a link, they can put that as <a href="...">...</a>, including setting _target if they want (which I think it's not possible now).

it is somewhat possible when running the custom parser on data and using the built in banner.

In my naivety, I thought this would be done just by setting el.innerHTML, and later https://developer.mozilla.org/en-US/docs/Web/API/Element/setHTML

@mariusa mariusa changed the title Allow HTML in announcement title Allow HTML in announcement content Dec 22, 2022
@ReeceM
Copy link
Owner

ReeceM commented Dec 22, 2022

When I made v2 it was designed to allow a person to pass there own rendering options into it through the template option. This was to allow custom HTML, I know it's not in the docs at the moment that might be what you are looking for? That allows for rendering a completely custom thing.

There is the option to actually provide a render class that can return the HTML you want written to the document. As mentioned, these came in on v2, but didn't document them yet.

@ReeceM
Copy link
Owner

ReeceM commented Dec 22, 2022

With using setHTML: I hadn't gone with the setHTML option initially I think cause of the support option, but I hopefully if there is some more browser compat I can slowly add that. It is one of the reasons why I gave provision for using DOMPurify, at least some could have the option. Given that it's on Chrome and some other browsers I'll look at adding it and then give a suggestion if it isn't that DOMPurify can be used.


it is somewhat possible when running the custom parser on data and using the built in banner.

^ the above isn't true now since the adjustment of the issue title. But the below explains more of what I was talking about.

Sorry, clarification on this, the parser is the part that takes what is defined in the templates that have {% json_key %}. The parser takes the HTML <template> and then renders those values and places it in using the el.innerHTML. This is what happens with the render class. Using the templates you can set things like _target.

@ReeceM
Copy link
Owner

ReeceM commented Dec 22, 2022

The attached image illustrates the area in red over the yellow that could have a users custom content placed in the default banner style. Is that the area you were expecting? Let me know, happy to see what can be done.

Frame 1

@mariusa
Copy link
Contributor Author

mariusa commented Dec 23, 2022

Thanks for clarifications, Reece!

The attached image illustrates the area in red over the yellow that could have a users custom content placed in the default banner style. Is that the area you were expecting?

Yes!

I think it would be useful to step back and clarify the high level goal of h-bar. IMHO, it focused on a very narrow use case: Load JSON from an URL to display a link (with implementation having a template engine included).

A larger use case would be: Display one or more announcements (one at a time), given as HTML content.

This would cover my use case and many others, including the original (but not forcing it). Examples:

  1. a simple text announcement ("Happy Holidays!")
  2. a link (what it does now)
  3. text with multiple links ('Please support us on Patreon or by purchasing the book')
  4. complex HTML (one-line newsletter signup form)

It's easy for the developer to use a templating engine if they wish (eg Handlebars) to produce that HTML, or get the content (JSON or HTML) from an API. These could even be in h-bar examples, but not bundled in the library.

If you think 'Display some HTML' is too simple, I'd still prefer to use a library instead of rolling my own. It needs to

  • look good on phones and desktops. This includes not only the margins, but maybe even font size
  • smart font sizes: larger for short announcements, smaller for long ones (try to fit on one line on phone/desktop)
  • be closable in a smart way: not show again the same announcement when navigating to another page, but another announcement which wasn't closed
  • have a flexible bar height depending on content (not limited to a fixed height) -- I guess this just works as long as height isn't hardcoded

I also liked the themes and badges support, which I wouldn't have thought about.

What do you think?

Implementation note:
The constructor could take an announcements attribute such as

[
  { content: "Happy holidays!" },
  { content: "Happy holidays!" },
  ...
]

Note that this doesn't directly provide a list of HTMLs, just in case each announcement might have some other options in future.

@ReeceM
Copy link
Owner

ReeceM commented Dec 26, 2022

Hi, thank you for the suggestions on the package. It was intended to be a simple solution initially, but I am open to extending it and happy to have this feedback.

I've been going over the suggestions above and will come back here to this thread, thank you for them, I wasn't ignoring them 👍

@mariusa
Copy link
Contributor Author

mariusa commented Dec 27, 2022

Thank you Reece! Happy holidays :)

@mariusa
Copy link
Contributor Author

mariusa commented Feb 24, 2023

Hi Reece, hope this finds you well. Curios to learn about your decision. Thanks

@ReeceM
Copy link
Owner

ReeceM commented Feb 24, 2023

Hi Marius, the message does find me well :) hope the same goes for you 👍

I just had a bit of a busy month and a bit pushing features out for a client but I'm happy with where this feature would go for the package.

@mariusa
Copy link
Contributor Author

mariusa commented Feb 24, 2023

Great, thanks Reece!

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

No branches or pull requests

2 participants