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

5-14-2024: Layout & Sample Created #691

Merged
merged 27 commits into from
May 20, 2024
Merged

Conversation

zdodson21
Copy link
Contributor

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.

Copy link

vercel bot commented May 14, 2024

@zdodson21 is attempting to deploy a commit to the elmsln Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

vercel bot commented May 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lrnwebcomponents ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 14, 2024 4:10pm

elements/media-quote/src/media-quote.js Show resolved Hide resolved
elements/media-quote/src/media-quote.js Outdated Show resolved Hide resolved
elements/media-quote/src/media-quote.js Outdated Show resolved Hide resolved
elements/media-quote/src/media-quote.js Outdated Show resolved Hide resolved
elements/media-quote/src/media-quote.js Outdated Show resolved Hide resolved
elements/media-quote/src/media-quote.js Outdated Show resolved Hide resolved
elements/media-quote/src/media-quote.js Outdated Show resolved Hide resolved
elements/media-quote/src/media-quote.js Outdated Show resolved Hide resolved
elements/media-quote/src/media-quote.js Outdated Show resolved Hide resolved
elements/media-quote/demo/index.html Outdated Show resolved Hide resolved
Copy link
Contributor

@btopro btopro left a 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

elements/media-quote/src/media-quote.js Outdated Show resolved Hide resolved
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>
Copy link
Contributor

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

Copy link
Contributor

@btopro btopro left a 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

elements/media-quote/src/media-quote.js Outdated Show resolved Hide resolved
elements/media-quote/src/media-quote.js Outdated Show resolved Hide resolved
<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>
Copy link
Contributor

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>

Copy link
Contributor Author

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?

Copy link
Contributor

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

@btopro btopro merged commit 9bedb38 into haxtheweb:master May 20, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants