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

Warn if there is a missing parent element #612

Merged
merged 7 commits into from
Oct 28, 2023
Merged

Conversation

adamghill
Copy link
Owner

@adamghill adamghill commented Oct 17, 2023

Log a message if:

  • there isn't a "wrapper" element (i.e. the component has more than 1 root element)
<!-- component-one.html -->
<input unicorn:model="first_name"></input>
<input unicorn:model="last_name"></input>
  • the root element doesn't have children
<!-- component-two.html -->
<input unicorn:model="first_name"></input>

I should also update the docs to make it more clear, but a "wrapper" element is required for the DOM diffing algorithm to work properly. These checks should hopefully help catch invalid component templates earlier.

@cyface
Copy link

cyface commented Oct 18, 2023

This would be very helpful to me!

@adamghill adamghill self-assigned this Oct 19, 2023
@adamghill
Copy link
Owner Author

@cyface How does this more explicit warning in the documentation with examples look?

image

@cyface
Copy link

cyface commented Oct 19, 2023

That is helpful! Again, not sure how to make painfully clear the difference between the component’s template and the outer template. 🤔

“There are typically two templates involved when using Unicorn. The component’s template, and the outer template where the application includes the component. If you need to render a component directly from a url rather than including it via template tag in an outer template, you can do that using Direct View.”

##Component Templates
Blah blah wrapper

Outer Templates

Blah blah unicorn tag

@adamghill
Copy link
Owner Author

Yeah, I think something like that could work. I'll continue to ruminate.

@rhymiz
Copy link
Contributor

rhymiz commented Oct 19, 2023

this is super helpful

…ew + template. Add warning about wrapper element. Explain convention for components.
@adamghill
Copy link
Owner Author

After re-reading the docs a few times, I realized that it doesn't explicitly make it clear that components are a Python view + HTML template. So, I explicitly split out a new views page and re-arranged a lot of the content. The overall goal is to reduce the head scratching moments that new users run into and generally clarify the concepts behind Unicorn.

@kevinbowen777
Copy link

line 154 in docs/source/views.md:

I noticed that you went to the effort of removing a "just" statement in templates.md. It might be worthwhile to do so here as well:

"Inheriting from unicorn.components.UnicornField is a quick way to serialize a custom class as it calls self.__dict__ under the hood."

@adamghill
Copy link
Owner Author

I noticed that you went to the effort of removing a "just" statement in templates.md. It might be worthwhile to do so here as well

Thanks for pointing that out @kevinbowen777. I've been trying to remove words like "just", "simple", "quick", etc. although I'm sure they are still more in there. 😄

I also just pushed some more changes mostly around rearranging the sidebar navigation to (hopefully) make things more clear.

@cyface
Copy link

cyface commented Oct 28, 2023

Typo in early part of the docs “forcing you to re-building your application”

@adamghill
Copy link
Owner Author

Typo in early part of the docs

Thanks! 🚀

@adamghill
Copy link
Owner Author

I'm going to merge this in, but any other feedback on doc changes would still be appreciated. 👍

@adamghill adamghill merged commit f497cdc into main Oct 28, 2023
5 checks passed
@adamghill adamghill deleted the warn-missing-parent branch October 28, 2023 19:54
adamghill added a commit that referenced this pull request Oct 28, 2023
will9288 added a commit to will9288/django-unicorn that referenced this pull request May 6, 2024
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.

4 participants