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

A Few Things... #28

Closed
5 of 7 tasks
skipjack opened this issue Jun 27, 2019 · 9 comments
Closed
5 of 7 tasks

A Few Things... #28

skipjack opened this issue Jun 27, 2019 · 9 comments

Comments

@skipjack
Copy link
Contributor

skipjack commented Jun 27, 2019

First I want to start by saying this library is fantastic, thanks for your work on it. Before using this lib, I tried regexes which ended up getting overly complex and slow. I'm happy to submit PRs for everything below, just wanted to talk through some of it through beforehand.

I noticed a few bugs so far...

These ones are necessary, but would be nice to have...

  • Expose getUniqAttr from plugin-helpers so preset authors and consumers can use it.

Uppercase Tags

For example, something like [B] kind of works in the demo because most browsers will automatically recognize and lowercase it (at least in the inspector) so it will still be rendered. That said the styling attributes you're adding with the b definition are lost and the demo only shows it bold because of default browser styling.

In react however, the following error is thrown:

<B /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.

To fix this issue, I simply injected a little preset before the main one to lowercase all tag names:

            plugins={[
                (tree, core) => {
                    tree.walk(node => {
                        if (typeof node === 'object' && node.tag) {
                            node.tag = node.tag.toLowerCase()
                        }

                        return node
                    })
                },
                preset()
            ]}

That works, but feels a little hacky so I'm thinking this probably makes sense as a built-in but I'm not sure where to put it.

@skipjack
Copy link
Contributor Author

skipjack commented Jun 28, 2019

Maybe reactPreset.extend does work but the example doesn't spread tags back in (...tags) like it does under the hood when extending presetHTML5? I used the example which excludes that spread...

  const preset = reactPreset.extend((tags, options) => ({
+   ...tags,
    quote: node => ({
      tag: "blockquote",
      content: node.content
    })
  }))

If this is the issue I'd be happy to submit a PR to update the docs.

UPDATE: That does seem to be the issue, I'll submit a PR (#29).

skipjack added a commit to skipjack/bbob that referenced this issue Jun 28, 2019
When using `@bbob/react`s `<BBCode>` component, the following error is thrown
if this change is not included...

```
Warning: Each child in a list should have a unique "key" prop.
```

Mentioned in JiLiZART#28
@skipjack
Copy link
Contributor Author

skipjack commented Jun 28, 2019

@JiLiZART take a look when you get a chance and let me know your thoughts. I'm pretty much done for now (have been integrating into a project it over the last day or two) but I'm still happy to help with these issues. I submitted PRs for the obvious fixes/updates but I wanted to hear what you had to say on the other things before moving forward.

Again, thanks for your work on this project.

@skipjack
Copy link
Contributor Author

Maybe I could apply the lowercase here? I don't know if there's any other implications, but in my experience all bbcode tags should be fine lower-cased.

@JiLiZART
Copy link
Owner

JiLiZART commented Jun 29, 2019

Maybe I could apply the lowercase here ? I don't know if there's any other implications, but in my experience all bbcode tags should be fine lower-cased.

Yes, TagNode is a good way to lowercase tag.

@JiLiZART
Copy link
Owner

@skipjack Thanks for contributing! Any help would be helpful.
I think this will make the project only better.

JiLiZART pushed a commit that referenced this issue Jun 29, 2019
When using `@bbob/react`s `<BBCode>` component, the following error is thrown
if this change is not included...

```
Warning: Each child in a list should have a unique "key" prop.
```

Mentioned in #28
@meodemsao
Copy link

meodemsao commented Aug 11, 2019

i use version 2.5.2

code sandbox

[ALT=http://media.thethaovanhoa.vn/2013/06/22/07/32/Pirlo.jpg]Pirlo[/ALT]

has error

Warning: <ALT /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements. in ALT (created by Component) in span (created by Component) in Component (created by App) in App

i seen #42 seem fix this issue ?

@JiLiZART
Copy link
Owner

I just published 2.5.3 you can try

@meodemsao
Copy link

meodemsao commented Aug 13, 2019

@JiLiZART thanks, it's work for me

@JiLiZART
Copy link
Owner

JiLiZART commented May 9, 2022

I freeze this due inactivity

@JiLiZART JiLiZART closed this as completed May 9, 2022
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

3 participants