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

Unescapable nested div class in AutoComplete #104

Open
qrilka opened this issue Feb 13, 2024 · 10 comments
Open

Unescapable nested div class in AutoComplete #104

qrilka opened this issue Feb 13, 2024 · 10 comments

Comments

@qrilka
Copy link
Contributor

qrilka commented Feb 13, 2024

AutoComplete provies class for its main div but internally it uses Input which adds an extra div inside and class for that input can't be changed through AutoComplete params

@luoxiaozero
Copy link
Collaborator

I can add an input_class parameter to the AutoComplete component.

@qrilka
Copy link
Contributor Author

qrilka commented Feb 18, 2024

There's attr:class which gets applied to input, basically right now we have:

<div> - with `class` available as `class` (`thaw-auto-complet` by default)
  <div> - with hard-coded `thaw-input` class
    <input> - with class available as `attr:class`

using input_class for 2nd level div would be confusing at least

@luoxiaozero
Copy link
Collaborator

Do you have any good ideas?

@qrilka
Copy link
Contributor Author

qrilka commented Feb 18, 2024

Not right now. I think the core problem here comes from this implicit nesting of Input inside of AutoComplete: ideally we should have only one div, it looks like on:keydown and class could be moved into Input but I'm not sure about auto_complete_ref what is it used for (BTW the guide doesn't quite tell much about it)?

@qrilka
Copy link
Contributor Author

qrilka commented Feb 19, 2024

One more detail: in Input there's a warning of one uses attr:class so I get in console:

Thaw: The 'class' attribute already exists on elements inside the Input component, which may cause conflicts.

basically it means that the third option (overriding input class with attr:class) isn't advised by the library at the moment.

@luoxiaozero
Copy link
Collaborator

but I'm not sure about auto_complete_ref what is it used for

auto_complete_ref is mainly used to get the DOM position in real time to display the popup position.

Also, can you write an example of your needs?

@qrilka
Copy link
Contributor Author

qrilka commented Feb 22, 2024

In my example I was trying to fit it into tailwind css classes written by another engineer. It's just a prototype so maybe we'll just throw away that piece of code soon.

@qrilka
Copy link
Contributor Author

qrilka commented Mar 24, 2024

@luoxiaozero could you help me finding PR/commit which fixed this? (cross-links could be helpful for cases like this BTW)

@luoxiaozero
Copy link
Collaborator

Sorry, I thought you no longer need this feature. I will reopen issue.

cross-links could be helpful for cases like this BTW

What does cross-links mean?

@luoxiaozero luoxiaozero reopened this Mar 28, 2024
@qrilka
Copy link
Contributor Author

qrilka commented Mar 28, 2024

with a cross-link I meant a link between a PR or commit and a ticket, e.g. Github finds those and adds to the ticket.

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

2 participants