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

Set cell-index attribute as a string #1317

Merged
merged 1 commit into from
Apr 13, 2023

Conversation

jtpio
Copy link
Member

@jtpio jtpio commented Apr 12, 2023

References

Currently Voila produces HTML with cell-index attributes set to integers:

image

This can for example be seen on Binder: https://mybinder.org/v2/gh/voila-dashboards/voila/main?urlpath=voila%2Ftree%2Fnotebooks

However attributes are more commonly set as string.

Also Voici expects a string when looking up these elements here: https://github.com/voila-dashboards/voici/blob/7b8825e10c05e31e4e1731fffbe60ffdc327304e/packages/voici/src/app.ts#L328

const element = document.querySelector(`[cell-index="${idx + 1}"]`);

Code changes

Update cell-index to be a string in the templates.

User-facing changes

None

Backwards-incompatible changes

None

@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch jtpio/voila/cell-index

@jtpio jtpio added this to the 0.5.0 milestone Apr 12, 2023
@trungleduc
Copy link
Member

Thanks @jtpio

@trungleduc trungleduc merged commit 103e2b9 into voila-dashboards:main Apr 13, 2023
@jtpio jtpio deleted the cell-index branch April 13, 2023 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants