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

AppRail improvements #1402

Merged
merged 11 commits into from
May 4, 2023
Merged

Conversation

unlocomqx
Copy link
Contributor

@unlocomqx unlocomqx commented Apr 29, 2023

Before submitting the PR:

  • Does your PR reference an issue? Discussion
  • Did you update and run tests before submission using npm run test?
  • Does your branch follow our naming convention? If not, please amend the branch name using branch -m new-branch-name
  • Did you update documentation related to your new feature or changes?

What does your PR address?

This PR improves DX related to using AppRail and AppRailTile.

  • You can omit tag="a" when you pass a href to AppRailTile.
  • href will be used as the value when no value is provided, resulting in less code repetition and simpler state management.
  • Add support for passing a readable to AppRail. Thus making it possible to pass a derived store (to pass in current pathname for example).
  • Fix a minor issue where an error is thrown after clicking on an AppRailTile without a store
    cannot read properties of undefined (reading 'set')

Code sample

<script>
  import { AppRail, AppRailTile } from "@skeletonlabs/skeleton"
  import { derived } from "svelte/store"
  import { page } from "$app/stores"

  const selected = derived(page, $page => $page.url.pathname)
</script>

<AppRail {selected}>
  <-- href is used as the value. tag prop is omitted !-->
  <AppRailTile href="/about" label="About"></AppRailTile>
  <AppRailTile href="/admin" label="Admin"></AppRailTile>
  <AppRailTile href="/login" label="Login"></AppRailTile>
</AppRail>

@vercel
Copy link

vercel bot commented Apr 29, 2023

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

Name Status Preview Comments Updated (UTC)
skeleton-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 4, 2023 5:50pm

@unlocomqx unlocomqx changed the title Feat/app rail improvements AppRail improvements Apr 29, 2023
@endigo9740
Copy link
Contributor

@unlocomqx just a heads up I did see this come through, but I'll have to return to this later this week when I have time for a proper review. Thanks for your submission. This looks great with a quick scan!

@unlocomqx
Copy link
Contributor Author

@endigo9740 It's cool, take your time

Thank you

@endigo9740
Copy link
Contributor

@unlocomqx this seems like a great change, great job. Thanks for your contribution. I'm going to merge this now and this will be part of next week's release!

@unlocomqx
Copy link
Contributor Author

Great news. Glad to have contributed to such awesome lib

Cheers

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.

None yet

2 participants