arobase-che / remark-attr

Remark plugin to add support for custom attributes
Other
58 stars 16 forks source link

Attribute for <p> / paragraph? #9

Open flikteoh opened 4 years ago

flikteoh commented 4 years ago

Hi,

I'm unsure if this is an issue of MDX or remark-attr.

I can't appear to use:

This is a paragraph.{.class-name}

I tried adding 'p' and 'paragraph' to the options.elemets as well.

Is this intended?

arobase-che commented 4 years ago

Not sure it should be considered as an issue.

paragraph are not in the list of remarkAttr.SUPPORTED_ELEMENTS.
Because paragraphs are simply not supported. ^^

flikteoh commented 4 years ago

As in there's no significant use cases for paragraph?

ie: This is some content in paragraph that displays in larger font size.{.larger}

Is there any possible way I can add support to paragraph by customising this plugin locally?

arobase-che commented 4 years ago

I will take some times to study it this WE but can't guaranty anything.

flikteoh commented 4 years ago

Alright. Thank you @arobase-che

arobase-che commented 4 years ago

Ok. I used to have studied that case.

Paragraph are a little special. They can't be implement the way other elements are. Because the attributes part {attr key=val #id .class} will be inside the paragraph unlike link/image, strong, ...

That doesn't mean it should not been supported but it a little more complicated. So I didn't close that issue.

flikteoh commented 4 years ago

Hi @arobase-che , thanks for your kind attention.

I have looked around and couldn't find a solution to this issue; and have moved on to using custom react components to deal with that.

Thanks for the effort though.

pe-s commented 4 years ago

@arobase-che before I try myself: Could I just fork and add 'p' to supportedElements and then get it working by putting the attr not besides (so inside the <p />) but on the next line like this:

This is a nice little paragraph.
{.with .some .extra .classes}

Should yield <p class='with some extra classes'>...

Could this work?

pe-s commented 4 years ago

Follow-up: it can't work because there is a line break in between, right?

arobase-che commented 4 years ago

Hum.

Basicaly. Nope it will not work. The problem should be easy to fix.

This is a nice little paragraph.
{.with .some .extra .classes}

Is a paragraph, the {.with ...} are inside the paragraph. So it doesn't work.

So this should work :

This is a nice little paragraph.

{.with .some .extra .classes}

But it doesn't. They is two newline between the paragraph and the extra part instead of one.

flikteoh commented 4 years ago

Previously when using Jekyll with GFM, the markdown format:

This is a paragraph.{:.classname}
{:.classname}
This is a paragraph.

Would both be working as intended.

Recently we have switched to Gatsby, trying to avoid wrapping custom components just to style texts.

Thus we tried remark-attr; so far we couldn't get it to work. (Including adding p to the supportedElements).

arobase-che commented 4 years ago

So, it looks like you really need it.

Here is just a PoC. The easy one, I talked about.

This is a nice little paragraph.

{.with .some .extra .classes}

Works on the paragraph branch. See : b15b4490ff50e8dd1b88393b9ca3e0f6b9b29f8e

I'm not sure it should be merged. Maybe we should code something a little more complex but that can deal with that case:

This is a nice little paragraph.
{.with .some .extra .classes}

PS: Everything is about syntax we want to support. I don't want to chose for others.

pe-s commented 4 years ago

@arobase-che's first example above could work in the wild. However it's a bit error prone and not intuitive for eg non-coders. 2nd example is intuitive and it's also good because the writer is forced to select an entire p that he can't span and put into some other div (compared to a same line aproach).

pe-s commented 4 years ago

Another reason against the first example: In some systems I split the markdown by paragraphs into an array to make the conversion to JSX only for that lines and not for the entire document (to make everything faster, basically an JIT-conversion of needed paragraphs).

arobase-che commented 4 years ago

I'm against too. I just don't have time to support the second example.

arobase-che commented 4 years ago

Ok, here is a new branch.

See fde4eca8e9372b19624de13e743a5c8b6c23c067, Should works has expected. If everything is fine, I will consider to merge it. Still need more tests. I think that it will be disabled by default and should be enabled manually.

janosh commented 4 years ago

@arobase-che Looks good. Would be a nice addition if merged. Along similar lines, how about supporting ul and ol as well? This

- li1
- li2
- li3
{.some .classes}

would give

<ul class="some classes">
  <li>li 1</li>
  <li>li 2</li>
  <li>li 3</li>
<ul>
cco3 commented 4 years ago

Is this still in the works?

jaguarondi commented 4 years ago

I tested kouhei-fuji's branch and it does work fine for me, also after merging with master. Is there anything anything I could do to help in order to merge this?