Skip to content

New: sharing state core concept!#934

Merged
bholmesdev merged 57 commits into
mainfrom
new/sharing-state
Jul 8, 2022
Merged

New: sharing state core concept!#934
bholmesdev merged 57 commits into
mainfrom
new/sharing-state

Conversation

@bholmesdev
Copy link
Copy Markdown
Contributor

@bholmesdev bholmesdev commented Jul 5, 2022

What kind of changes does this PR include?

  • Minor content fixes (broken links, typos, etc.)
  • New or updated content
  • Translated content
  • Changes to the docs site code
  • Something else!

Description

  • Add new Tabs Preact component for tab views across our docs. This includes:
    • A UIFrameworkTabs utility component for easily embedding UI framework comparisons
    • A generic Tabs component for any tab view you could want, based on slots
  • Add a new core concept page: Sharing State. This includes:
    • A primer on what user problem this page addresses
    • Why nanostores (with a brief comparison to Svelte stores)
    • A usage example that will mirror one of our astro.new examples soon
    • A walkthrough on nanostore "atoms"
    • A walkthrough on nanostore "maps"
  • Add LoopingVideo.astro utility for video embeds
sharing-state-docs.mov

@netlify
Copy link
Copy Markdown

netlify Bot commented Jul 5, 2022

Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit e175c1b
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/62c897ad6753f40008657bf7
😎 Deploy Preview https://deploy-preview-934--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.

@crutchcorn
Copy link
Copy Markdown
Contributor

Awesome work! 🎉

Something I'm noticing is the "Jump when tab switch" due to a mismatch of content length between tabs. This leads to a jarring experience that, when I implemented it for my blog, lead to a lot of errant bug reports being filed.

Here's a GIF of this in an extreme example:

Screen.Recording.2022-06-19.At.2.41.02.Pm-1.mp4

To fix this, I effectively:

  • Attached a unique ID to each tab header
  • On tab selection change, added a setTimeout(() => {}, 0) to queue a microtask to jump to the tab header using scrollIntoView

Here's a PR that I wrote to fix this problem:

playfulprogramming/playfulprogramming#374

Copy link
Copy Markdown
Member

@yanthomasdev yanthomasdev left a comment

Choose a reason for hiding this comment

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

Amazing work Ben "Tabs" H! I left a few suggestions for you 🙌

I also verified the Tabs component accessibility with NVDA and it is perfect for me 👌

Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
Comment thread src/i18n/en/nav.ts Outdated
Comment thread src/components/tabs/Tabs.module.css Outdated
Comment thread src/pages/en/core-concepts/sharing-state.md
@zadeviggers
Copy link
Copy Markdown
Contributor

This looks great! I did notice that on mobile (android firefox) some of the tabs overflow off the page and you can't scroll them to see the cut off content.
I also think it might be good to touch on the Solid stores a bit more in depth since the page is about sharing state in general, not nanostores specificaly (unless I'm very much mistaken, in which case I apologise).
Overall this is a very well written page!

@yanthomasdev yanthomasdev added add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. site improvement Some thing that improves the website functionality - ask @delucis for help! labels Jul 6, 2022
Copy link
Copy Markdown
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

Looking great! Two small suggestions, neither blocking.

Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
Comment thread src/pages/en/core-concepts/sharing-state.md
@bholmesdev
Copy link
Copy Markdown
Contributor Author

@zadeviggers horizontal scrolling added!

@bholmesdev
Copy link
Copy Markdown
Contributor Author

@zadeviggers Also added a callout on solid stores for shared state. Let me know if these instructions make sense!
e8123d1

@bholmesdev
Copy link
Copy Markdown
Contributor Author

@crutchcorn addressed!

@zadeviggers
Copy link
Copy Markdown
Contributor

Looks great @bholmesdev!

@sarah11918
Copy link
Copy Markdown
Member

Just a quick note before I check the sites I have queued up to read re: "using emoji as end punctuation".... BEN...

I'm not sure this page fits in the "Core Concepts" section, and I'm leaning towards it belonging in "Features" rather than "Basics" (as the pages are structured now). I'm curious to hear others' thoughts on this.

The material that Fred is drafting in #893 is more overall description/philosophy of principle, and not technical walkthroughs. "Basics" are (right now) for things that are inherent to understanding an Astro project: file structure, components, pages/routing etc. and "Features" (formerly "Guides") are "Things you'd expect a website to have/do, and here's how you'd do that in/with your Astro site." The Basics briefly show some essential code patterns, but the Features will demonstrate the code required to achieve ... a feature. This feels like the latter to me? Thoughts?

Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
@hippotastic
Copy link
Copy Markdown
Contributor

I love the general idea of this! However, switching any of the tabs in the Netlify preview doesn't work on iOS Safari for me.

@hippotastic
Copy link
Copy Markdown
Contributor

It works well in iOS Safari now after your updates. Great job!

@FredKSchott
Copy link
Copy Markdown
Member

Like @sarah11918 said, this is more of a Guide than a technical concept essential to the very core of Astro :) Place it with the other features in the sidebar, and we can experiment with renaming that heading to "Guides" if it makes more sense.

Copy link
Copy Markdown
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Hey Ben! Well isn’t this fantab ulous! Thank you. Sorry it took me a while to get you a review.

I’ve mostly focused on the technical side of things as I’m sure Sarah will have things to say about the content, and I don’t want to overwhelm you with too many editors.

Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
Comment thread src/pages/en/core-concepts/sharing-state.md
Comment thread src/components/tabs/Tabs.tsx
Comment thread src/components/tabs/Tabs.module.css
Comment thread src/components/tabs/Tabs.tsx Outdated
Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Whew! Ben! Sorry, this took a little long because besides just a proofreading edit, one thing I look closely for is internal consistency both within the page and within the Docs as a whole. And, this, while delightful, takes both a slightly different tone, and a different level of technical explanation than most other pages in the docs. So, it did also require a bit of double-checking for "comps" around the site... all while reading the comments in Discord, seeing the file update underneath me...

So, here are my thoughts! Thanks for being patient! And thanks as always for contributing to the docs.. and uniting so many people into one PR. It's great to see so many people get involved!

Comment thread src/pages/en/core-concepts/sharing-state.md Outdated

"Typical" UI frameworks like React, Vue, or Svelte may encourage ["context" providers](https://reactjs.org/docs/context.html) for other components to consume. But when [partially hydrating components](/en/core-concepts/framework-components/#hydrating-interactive-components) within Astro or Markdown, you cannot use these context wrappers 😳

We'll need a different solution to create shared stores your components can read and write from: [**nanostores**](https://github.com/nanostores/nanostores).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
We'll need a different solution to create shared stores your components can read and write from: [**nanostores**](https://github.com/nanostores/nanostores).
We'll need a different solution to create shared stores your components can read from and write to: [**nanostores**](https://github.com/nanostores/nanostores).

nit: prepositions

Also, this does somewhat frame nanostores as the solution (even though you back off on that below).

An alternative, if you cared, is: "Astro suggests a different solution to ..."

Then, the question "Why nanostores?" flows well, as if to answer, "Why does Astro recommend that?". The list of alternatives following that again fits with Astro "suggesting" but not requiring, and demonstrating other options.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated to the clearer: "Astro recommends a different solution for shared client-side storage"

Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
Comment thread src/pages/en/core-concepts/sharing-state.md Outdated
</Fragment>
</UIFrameworkTabs>

Now, you should have a fully interactive ecommerce example with the smallest JS bundle in the galaxy 🚀
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

AP style guide says "e-commerce." Shopify, WordPress and Amazon use ecommerce. ... in our Astro vs X page we say eCommerce... 🤪

I'm OK with ecommerce, and will make a note to update elsewhere. Just wanted it to be known that I checked, for the record!

bholmesdev and others added 25 commits July 8, 2022 16:46
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
@bholmesdev bholmesdev force-pushed the new/sharing-state branch from 8061f18 to e175c1b Compare July 8, 2022 20:46
@bholmesdev bholmesdev merged commit 6916c3c into main Jul 8, 2022
@bholmesdev bholmesdev deleted the new/sharing-state branch July 8, 2022 20:55
@sarah11918
Copy link
Copy Markdown
Member

Thanks for fi-NIT-shing this off, Ben! Friday merge, FTW! 🥳

@inwardmovement
Copy link
Copy Markdown
Contributor

inwardmovement commented Jul 9, 2022

Were Alpine stores considered? May we add comparison/examples with them?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. site improvement Some thing that improves the website functionality - ask @delucis for help!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants