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

fix!: solara.Meta elements insert empty span into DOM #625

Conversation

iisakkirotko
Copy link
Collaborator

@iisakkirotko iisakkirotko commented Apr 30, 2024

The insertion of <span></span> into the dom is what causes strange extra spacing at the top of notebook-based tutorials with solara.Meta tags currently:

image

Fixes #626

TODO:

  • Revert change to jupyter dashboard tutorial page

Copy link
Collaborator Author

iisakkirotko commented Apr 30, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@mariobuikhuizen
Copy link
Contributor

Why is this a problem? Why is the span even needed?

@iisakkirotko
Copy link
Collaborator Author

@mariobuikhuizen This is what causes the extra spacing at the top of notebook-based tutorials with solara.Meta tags currently:

image

Granted this happens because these spans get inserted into a flex-box with some gaps defined, so not using that could be an alternative solution. The spans seem to be necessary because Vue does not like empty templates, those throw two errors saying:

[Vue warn]: Error compiling template:
Cannot use <template> as component root element because it may contain multiple nodes.

and

[Vue warn]: Error compiling template:
Templates should only be responsible for mapping the state to the UI. Avoid placing tags with side-effects in your templates, such as <script>, as they will not be parsed.

@mariobuikhuizen
Copy link
Contributor

@mariobuikhuizen This is what causes the extra spacing at the top of notebook-based tutorials with solara.Meta tags

This would be a better title for the commit/PR.

The spans seem to be necessary because Vue does not like empty templates, those throw two errors

Strange, I never noticed an error with an empty template.

@mariobuikhuizen
Copy link
Contributor

Ok, I checked it, I write empty templates like this:

<template>
</template>

This produces no errors.

Doing it like this:

<template></template>

Does produce the second error you mentioned.

@iisakkirotko iisakkirotko force-pushed the 04-30-fix_docs_wide_outputs_of_jupyter_notebook_based_tutorials_overflowing_width_limit_ branch from e500a9b to 103e0af Compare May 1, 2024 11:06
@iisakkirotko iisakkirotko force-pushed the 04-30-fix_docs_solara.meta_elements_insert_empty_span_into_dom branch from bc6dd96 to 432d4b6 Compare May 1, 2024 11:06
@mariobuikhuizen
Copy link
Contributor

mariobuikhuizen commented May 1, 2024

It's a bug in ipyvue that empty template tags on the same line don't work.

@iisakkirotko
Copy link
Collaborator Author

Putting the templates on two different lines fixes both errors. What are your thoughts on the PR? In principle this is a breaking change on the UI side (if someone was unknowingly relying on this for spacing, for example), but I think we can sell it as a fix. I could rename it as suggested, but then the scope of the change seems smaller than it might be.

@iisakkirotko iisakkirotko added this to the Solara 2.0 milestone May 1, 2024
@iisakkirotko iisakkirotko force-pushed the 04-30-fix_docs_solara.meta_elements_insert_empty_span_into_dom branch from 432d4b6 to ffbdeff Compare May 1, 2024 12:09
@iisakkirotko iisakkirotko changed the base branch from 04-30-fix_docs_wide_outputs_of_jupyter_notebook_based_tutorials_overflowing_width_limit_ to master May 1, 2024 12:09
mariobuikhuizen added a commit to widgetti/ipyvue that referenced this pull request May 1, 2024
Having an empty template on one line or no tempate tag at all would
incorrectly fallback to the deprecated template format where template
tags were ommited.

See: spacetelescope/jdaviz#2668
and widgetti/solara#625
mariobuikhuizen added a commit to widgetti/ipyvue that referenced this pull request May 2, 2024
Having an empty template on one line or no tempate tag at all would
incorrectly fallback to the deprecated template format where template
tags were ommited.

See: spacetelescope/jdaviz#2668
and widgetti/solara#625
@iisakkirotko iisakkirotko changed the title fix[docs]: solara.Meta elements insert empty span into DOM fix: solara.Meta elements insert empty span into DOM Jul 4, 2024
@iisakkirotko iisakkirotko changed the title fix: solara.Meta elements insert empty span into DOM fix!: solara.Meta elements insert empty span into DOM Jul 4, 2024
@maartenbreddels maartenbreddels force-pushed the master branch 2 times, most recently from 86646f5 to df2fd66 Compare July 11, 2024 15:41
@maartenbreddels maartenbreddels force-pushed the master branch 2 times, most recently from cded5b2 to 32af76f Compare December 20, 2024 12:52
@iisakkirotko iisakkirotko force-pushed the 04-30-fix_docs_solara.meta_elements_insert_empty_span_into_dom branch from ffbdeff to 98f59f9 Compare December 23, 2024 09:14
@iisakkirotko iisakkirotko changed the base branch from master to 12-23-feat_change_mutation_detection_and_allow_reactive_boolean_defaults December 23, 2024 09:14
@iisakkirotko iisakkirotko mentioned this pull request Dec 23, 2024
5 tasks
@iisakkirotko iisakkirotko merged commit 3a90256 into 12-23-feat_change_mutation_detection_and_allow_reactive_boolean_defaults Dec 23, 2024
29 of 30 checks passed
iisakkirotko added a commit that referenced this pull request Dec 23, 2024
…OM (#625)

* fix[docs]: solara.Meta elements insert empty span into DOM

* chore: remove workaround from jupyter notebook tutorial

Reverts #627, since the workaround is no longer needed.

* fix!: navigator would insert an empty span into dom
iisakkirotko added a commit that referenced this pull request Dec 27, 2024
…OM (#625)

* fix[docs]: solara.Meta elements insert empty span into DOM

* chore: remove workaround from jupyter notebook tutorial

Reverts #627, since the workaround is no longer needed.

* fix!: navigator would insert an empty span into dom
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

Successfully merging this pull request may close these issues.

solara.Meta adds an empty span to DOM
2 participants