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

The draggable attribute isn't handled right #2975

Closed
dharmax opened this issue Dec 18, 2022 · 7 comments
Closed

The draggable attribute isn't handled right #2975

dharmax opened this issue Dec 18, 2022 · 7 comments

Comments

@dharmax
Copy link

dharmax commented Dec 18, 2022

The issue:

  1. The draggable attribute isn't handled right by riot when you assign it a boolean expression.
  2. The HTML standard (and the browser) expects values of "true" or "false" (unlike, "checked", for example, which expects "checked" value or non).
  3. Riot translate a truthy value into the string "draggable" - which doesn't make it draggable.

Reproduction
https://codesandbox.io/embed/riot-js-7-bug-template-forked-1mco0f?fontsize=14&hidenavigation=1&theme=dark

Test context
browsers: Chrome and Firefox, linux OS, riot version 7

Issue type
Bug

@GianlucaGuarini
Copy link
Member

Changing this behavior is technically a breaking change. However according to the Riot.js documentation

Riot automatically detects all the valid HTML boolean attributes.

At moment it's not yet the case so I guess we can fix it in a minor version update.

@exside
Copy link

exside commented Mar 20, 2023

Well, if anyone would have used a boolean, the implementation wouldn't work anyways right? So would it really be breaking if it didn't work in the first place?

@GianlucaGuarini
Copy link
Member

This issue was already fixed by riot/compiler#166. A new Riot.js major release will be released soon.

@GianlucaGuarini
Copy link
Member

Fixed in riot@9.0.0

@drnkwati
Copy link

Did riot skip version numbers?
As per my understanding, riot is presently on version 4. Why is the next release tagged as v9?

@GianlucaGuarini
Copy link
Member

Hi @dharmax I thought It was clear from the release notes. V9 is used to sync all the ecosystem packages under the same version. Aren't the release notes clear enough? :)

@dharmax
Copy link
Author

dharmax commented Aug 1, 2023

Hi, bro, i think perhaps you were trying to answer @drnkwati 's question, not mine...?

Hi @dharmax I thought It was clear from the release notes. V9 is used to sync all the ecosystem packages under the same version. Aren't the release notes clear enough? :)

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

No branches or pull requests

4 participants