RpNation / bbcode

RpNation's Official BBCode Implementation for Discourse
GNU General Public License v3.0
1 stars 3 forks source link

BBCode Header added. #65

Closed kyuukestu closed 5 months ago

kyuukestu commented 5 months ago

BB Code Header tag added.

Stylizes text into a header format.

Example:

[h]Content[/h]

Alteras1 commented 5 months ago

Hmm. @MShultz do we want to care about adding ARIA roles to them? Ideally these should just be straight translations to some H1 or something, but I don't want to start messing what ppl might be styling with.

https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/heading_role

MShultz commented 5 months ago

Hmm. @MShultz do we want to care about adding ARIA roles to them? Ideally these should just be straight translations to some H1 or something, but I don't want to start messing what ppl might be styling with.

https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/heading_role

If we keep this structure that an [h] tag is a div wrapper with our styles, it would be a good idea to add the ARIA roles to it. tags already have that inherently.

@Alteras1 For consistency with the platform we're rolling to, how do you feel about the [h] tag mirroring the Markdown ## structure? So tags like [h1] [h2] [h3], etc(with [h] mirroring [h1] for backwards compatibility) that would then compile down to <h1> <h2>, etc. You can still assign their own class stylings if we want them to mirror what's on the live site, but that I think is closer to MDNs suggested best practices if I remember correctly.

kyuukestu commented 5 months ago

Looks good! Just one very small ask on an alphebetisation miss. It looks like there are some merge conflict with main as well, could you look into resolving those for this branch?

There's some mention of a conflict in bbcode-src/preset.js but I haven't the slightest idea where that conflict comes from. I've reviewed the changes to the file and don't see anything that could have caused the conflict.

kyuukestu commented 5 months ago

@MShultz The conflict has been resolved by deleting a few lines of input. I...would review it again before committing, but I assume the preset.js in /main was updated sometime after I cloned the repository.

MShultz commented 5 months ago

Hmm. @MShultz do we want to care about adding ARIA roles to them? Ideally these should just be straight translations to some H1 or something, but I don't want to start messing what ppl might be styling with. https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/heading_role

If we keep this structure that an [h] tag is a div wrapper with our styles, it would be a good idea to add the ARIA roles to it. tags already have that inherently.

@Alteras1 For consistency with the platform we're rolling to, how do you feel about the [h] tag mirroring the Markdown ## structure? So tags like [h1] [h2] [h3], etc(with [h] mirroring [h1] for backwards compatibility) that would then compile down to <h1> <h2>, etc. You can still assign their own class stylings if we want them to mirror what's on the live site, but that I think is closer to MDNs suggested best practices if I remember correctly.

@kyuukestu, what are your thoughts on this suggestion to refactor the headers so they mirror Markdown a little more. @Alteras1 , let me know if you disagree with these reqs as I outline them below:

Refactor the header and subheader tag to mirror Markdown header support.

For backward compatibility:

  1. [h] tag should render to an <h1> HTML element.
  2. [sh] tag should render to an <h2> HTML element

For additional options and mirroring Markdown, these would all be newly supported header bbcodes:

  1. [h1] tag should render to an <h1> HTML element.
  2. [h2] tag should render to an <h2> HTML element.
  3. [h3] tag should render to an <h3> HTML element.
  4. [h4] tag should render to an <h4> HTML element.
  5. [h5] tag should render to an <h5> HTML element.
  6. [h6] tag should render to an <h6> HTML element. These suggested changes would also affect the work in #68.
kyuukestu commented 5 months ago

Hmm. @MShultz do we want to care about adding ARIA roles to them? Ideally these should just be straight translations to some H1 or something, but I don't want to start messing what ppl might be styling with. https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/heading_role

If we keep this structure that an [h] tag is a div wrapper with our styles, it would be a good idea to add the ARIA roles to it. tags already have that inherently. @Alteras1 For consistency with the platform we're rolling to, how do you feel about the [h] tag mirroring the Markdown ## structure? So tags like [h1] [h2] [h3], etc(with [h] mirroring [h1] for backwards compatibility) that would then compile down to <h1> <h2>, etc. You can still assign their own class stylings if we want them to mirror what's on the live site, but that I think is closer to MDNs suggested best practices if I remember correctly.

@kyuukestu, what are your thoughts on this suggestion to refactor the headers so they mirror Markdown a little more. @Alteras1 , let me know if you disagree with these reqs as I outline them below:

Refactor the header and subheader tag to mirror Markdown header support.

For backward compatibility:

  1. [h] tag should render to an <h1> HTML element.
  2. [sh] tag should render to an <h2> HTML element

For additional options and mirroring Markdown, these would all be newly supported header bbcodes:

  1. [h1] tag should render to an <h1> HTML element.
  2. [h2] tag should render to an <h2> HTML element.
  3. [h3] tag should render to an <h3> HTML element.
  4. [h4] tag should render to an <h4> HTML element.
  5. [h5] tag should render to an <h5> HTML element.
  6. [h6] tag should render to an <h6> HTML element. These suggested changes would also affect the work in Code/subheader #68.

My approach to this is, you say jump, I saw how high. I'm here to learn and offer what I can ^^

That said, would you then prefer all the headers together in one file? And if we do go down to h6, is there any preference for the stylings applied?

Alteras1 commented 5 months ago

I'm fine with those mappings. Let's keep it plain for the time being, no extra styling. I think discourse already has its own stylings for those heading levels, so it should naturally adopt those if we don't do anything.

MShultz commented 5 months ago

I agree! Let's keep the stylings plain. You should be good to write out all the header tags in one file since they're all so similar. It'll help reduce our number of files and keep things tidy. :) Thank you!