Skip to content

Polished component and especially slot content#397

Merged
sarah11918 merged 7 commits intomainfrom
astro-components-esp-slots
May 4, 2022
Merged

Polished component and especially slot content#397
sarah11918 merged 7 commits intomainfrom
astro-components-esp-slots

Conversation

@sarah11918
Copy link
Copy Markdown
Member

Team effort, to revamp the slot material to address some slot issues raised in the community re: mixing with other frameworks.

Also, added some new touches re: our fancy new directives reference! (So... check that!)

@netlify
Copy link
Copy Markdown

netlify bot commented Apr 28, 2022

Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit c9aa083
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/627298f02e360e0009aa1711
😎 Deploy Preview https://deploy-preview-397--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sarah11918
Copy link
Copy Markdown
Member Author

@Jutanium Here are the fruits of our orangey labours! (Where are our socks, indeed?)

@Jutanium
Copy link
Copy Markdown
Contributor

🧦 🧦 ❤️

Copy link
Copy Markdown
Contributor

@aFuzzyBear aFuzzyBear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few thoughts, Ill be back , need to rest eyes,

@sarah11918
Copy link
Copy Markdown
Member Author

sarah11918 commented Apr 29, 2022

Quick note, since I'm not yet sitting down to really tackle this, just to give some context to steer the convo by the time I get there: 😄

Based on feedback/issues re: using slots with other framework components that also have a concept of slots, @Jutanium and I tried to reorganize the logical flow of material to see if that would help preempt that particular problem. Essentially:

FROM: slots are placeholders, components with slots are wrappers

TO: some components can have "children" html elements that get passed to them; that stuff goes in to a component's slot. (and, eg, that's why showing the file using the component first, THEN the component with the slot in the code examples.)

So, that's where these changes came from, but of course, I want the explanation to work just in general, and not to be optimized for one thing we can see coming.

Love that everyone's looking at this so critically, and if general consensus is that changing this section's logic is NOT preferred, we can go back to the original for now, and the drawing board for later. That's an option, too! (I still want the other changes sprinkled throughout the file for polish, though!)

@sarah11918 sarah11918 added the improve or update documentation Enhance / update existing documentation (e.g. add example, improve description, update for changes) label Apr 30, 2022
@sarah11918
Copy link
Copy Markdown
Member Author

OK, @Jutanium @delucis @aFuzzyBear ... I'm "slotting" you in for another round!

Can I suggest, DON'T revisit the comments above, and DO start reading at "Slots" to see, with fresh eyes and standing on its own, what you think??

@Jutanium
Copy link
Copy Markdown
Contributor

Jutanium commented May 4, 2022

Let's Get That Merged

Copy link
Copy Markdown
Contributor

@aFuzzyBear aFuzzyBear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is better, 🙂

@sarah11918
Copy link
Copy Markdown
Member Author

🥳 Let he who complains, open the next PR!

@sarah11918 sarah11918 merged commit 0212a8c into main May 4, 2022
@sarah11918 sarah11918 deleted the astro-components-esp-slots branch May 4, 2022 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improve or update documentation Enhance / update existing documentation (e.g. add example, improve description, update for changes)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants