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

Speed #431

Merged
merged 3 commits into from
Nov 18, 2022
Merged

Speed #431

merged 3 commits into from
Nov 18, 2022

Conversation

jjagielka
Copy link
Contributor

📑 Description

Speed Dial component. All functionality implemented, although doc page more condensed.

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • I have checked the page with https://validator.unl.edu/
  • All the tests have passed
  • My pull request is based on the latest commit (not the npm version).

ℹ Additional Information

@vercel
Copy link

vercel bot commented Nov 17, 2022

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

Name Status Preview Updated
flowbite-svelte ✅ Ready (Inspect) Visit Preview Nov 18, 2022 at 10:34PM (UTC)
flowbite-svelte-update ✅ Ready (Inspect) Visit Preview Nov 18, 2022 at 10:34PM (UTC)

@shinokada
Copy link
Collaborator

This could be the Flowbite problems, when I check with A11y I get slot and aria-controls warnings.

What do you think?

@jjagielka
Copy link
Contributor Author

I've noticed that as well. I've just copied aria-controls from Flowbite.com. Would be worth checking with them.

I think we can remove it for now and add it back when the issue is cleared out.

@shinokada
Copy link
Collaborator

shinokada commented Nov 18, 2022

  1. I think we should remove all the slot attributes.

  2. How about adding id prop?
    Use the id for aria-controls and {id} for Popper.

// SpeedDial.svelt
<script context="module" lang="ts">
...
  let id:string=''
</script>
...
...
<div class={divClass}>
  <Button
    {pill}
    name="Open actions menu"
    aria-controls="{id}"
    aria-expanded="false"
    color="blue"
    class="!p-3">
    <slot name="icon">
      <svg
        aria-hidden="true"
        class="w-8 h-8 transition-transform group-hover:rotate-45"
        fill="none"
        stroke="currentColor"
        viewBox="0 0 24 24"
        xmlns="http://www.w3.org/2000/svg"
        ><path
          stroke-linecap="round"
          stroke-linejoin="round"
          stroke-width="2"
          d="M12 6v6m0 0v6m0-6h6m-6 0H6" /></svg>
    </slot>
    <span class="sr-only">Open actions menu</span>
  </Button>
  <Popper {id} {trigger} arrow={false} color="none" activeContent {placement} class={poperClass}>
    <slot />
  </Popper>
</div>

@jjagielka
Copy link
Contributor Author

ad 1. What do you mean by ’remove all the slot attributes’?

@shinokada
Copy link
Collaborator

Since svg doesn't have slot attribute, we should remove slot="icon".

@shinokada
Copy link
Collaborator

shinokada commented Nov 18, 2022

Well I noticed that it is Svelte slot, so I'm not sure. We just ignore?

@jjagielka
Copy link
Contributor Author

slot="name" is used to replace the default + sign inside the blue button to some other svg: see the "dropdown menu" or "alternative menu" examples where "+" is replaced by "pen" or three dots "...".

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