-
Notifications
You must be signed in to change notification settings - Fork 87
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
5-14-2024: Layout & Sample Created #691
Conversation
@zdodson21 is attempting to deploy a commit to the elmsln Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
two small changes; otherwise looking good, I'll merge and test locally after you fix these up
alt="Old Main in front of a blue sky" | ||
> | ||
<span slot="quote">"Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Metus vulputate eu scelerisque felis. Nisl nisi scelerisque eu ultrices vitae auctor eu. Id leo in vitae turpis. Eu facilisis sed odio morbi quis commodo."</span> | ||
<span slot="author">John Doe</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
div
all the things since span is inline for slot elements here. I'd also make a default one that doesn't require being marked up like the quote one just being any general div
content. The others look good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I lied, added a few more things
<span class="content"><slot name="quote"></slot></span> | ||
${this.querySelector('[slot="author"]') && this.querySelector('[slot="author"]').textContent.trim().length > 0 ? html` | ||
<span class="citation"> | ||
<span class="author">- <slot name="author"></slot></span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make a field for author as well like <slot name="author">${this.author}</slot>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume I would need to define an author variable in constructor()
and properties()
, and do something similar with other slots?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it was to be 1000% stateful then you'd also recheck the variable using a mutation observer but that's a bit intense for how simple this is. Making the var reactive is a good idea s you suggest. Do the evaluation in constructor. If you look at the QuestionElement
base class I am making it has something like this as a pattern to follow
Created a hard coded component, which still needs some modification for various screen size compatability (both zooming in browser and mobile devices). Attribute based primary and accent system is not yet implemented, hope to begin working on that 5-15-2024 after ensuring the component stays consistent in visual style at various screen sizes.