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

Attribute for <p> / paragraph? #9

Open
flikteoh opened this issue Oct 23, 2019 · 18 comments
Open

Attribute for <p> / paragraph? #9

flikteoh opened this issue Oct 23, 2019 · 18 comments

Comments

@flikteoh
Copy link

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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

@flikteoh
Copy link
Author

Alright. Thank you @arobase-che

@arobase-che
Copy link
Owner

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
Copy link
Author

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
Copy link

pe-s commented Feb 24, 2020

@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
Copy link

pe-s commented Feb 24, 2020

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

@arobase-che
Copy link
Owner

arobase-che commented Feb 24, 2020 via email

@flikteoh
Copy link
Author

flikteoh commented Feb 25, 2020

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
Copy link
Owner

arobase-che commented Feb 25, 2020

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 : b15b449

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
Copy link

pe-s commented Feb 25, 2020

@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
Copy link

pe-s commented Feb 25, 2020

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
Copy link
Owner

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

arobase-che added a commit that referenced this issue Feb 25, 2020
@arobase-che
Copy link
Owner

Ok, here is a new branch.

See fde4eca, 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
Copy link

janosh commented Mar 31, 2020

@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>

kouhei-fuji pushed a commit to kouhei-fuji/remark-attr that referenced this issue Apr 8, 2020
@cco3
Copy link

cco3 commented Jun 21, 2020

Is this still in the works?

@jaguarondi
Copy link

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?

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

No branches or pull requests

6 participants