bennymi / ato-ui

The elemental UI component library for Svelte, built with UnoCSS and Melt UI.
https://ato-ui.vercel.app
MIT License
70 stars 4 forks source link

Enhance Switch #144

Open Mohamed-Kaizen opened 9 months ago

Mohamed-Kaizen commented 9 months ago

Describe the feature.

Hi,

The current styled switch has some problems with RTL (right-to-left) settings, and I think we can also improve its appearance.

Switch _ Ato UI (1)

Add examples

repl. This example does not use Unocss or Melt UI, but it solves the RTL issue and adds more style to the switch. If you think this switch is acceptable, I can build it using Unocss and Melt UI, and send a pull request.

What kind of request is it related to?

Enhancement

bennymi commented 9 months ago

@Mohamed-Kaizen I love your switches. Definitely much nicer than the current ones. It'd be awesome to integrate them into the library, so go for it. :)

It might also be nice to have a prop to control the animation speed if that's possible. What do you think? Maybe have 3 different options if we want to limit it. And having the possibility of having the icons inside the sliding button or in the background is also pretty cool actually, like in that first example of yours.

Just make sure that when you update the switch you keep using the relveant tokens: bg-primary/surface and rounded-btn, etc. These might change in the future while I'm updating the designer, but for now they're the relevant ones.

Mohamed-Kaizen commented 9 months ago

before i send PR, i created playground to get your feedback

Just make sure that when you update the switch you keep using the relveant tokens: bg-primary/surface and rounded-btn, etc. These might change in the future while I'm updating the designer, but for now they're the relevant ones.

I used the theme in my playground. Any suggestion of what i miss or need to add ?

It might also be nice to have a prop to control the animation speed if that's possible. What do you think? Maybe have 3 different options if we want to limit it.

We could use variables like --switch-animation, --switch-animation-duration, etc. to control the animation speed. This would allow users to modify the animation speed without touching other transition properties, but it would also mean that they would have to remember a lot of --something variables.

Alternatively, we could use props as you suggested like bgAnimation, textAnimation, and circleAnimation. This would mean that users would only need to remember three props, but they would need to copy and paste the rest of the transition properties if they only want to change one property.


what do you think ?

bennymi commented 9 months ago

@Mohamed-Kaizen this looks great.

A few comments:

For the inactive and active background color use the props like in the old switch. Then you can add them to the element classes. Use bg-primary-500 as the default active color and some surface color that looks nice in dark and light mode on the website (bg-surface-300-400 for example).

The shape of the switch will be defined by the theme, so for the roundedness use the rounded-btn class instead of rounded-3xl. I'm currently updating the theme designer, so in the future there will be a separate class for it, so that buttons and switches don't have to be the same shape, but I'll update that later on.

We could use variables like --switch-animation, --switch-animation-duration, etc. to control the animation speed. This would allow users to modify the animation speed without touching other transition properties, but it would also mean that they would have to remember a lot of --something variables.

Alternatively, we could use props as you suggested like bgAnimation, textAnimation, and circleAnimation. This would mean that users would only need to remember three props, but they would need to copy and paste the rest of the transition properties if they only want to change one property.

I was thinking we could have a transitionDurations prop, where all default duration times can be overwritten. If you want to change one duration time you probably need to change all the others as well, so it makes sense to keep them in one object, but since all keys are optional they can also just set one if they want. I think we only need to have the duration-... classes in there. This way users can also turn the durations off if they want with duration-0. Something like this:

types.ts

// all values are optional for this type
export type SwitchTransitionDurations = {
    icon?: string;
    onOffSlots?: string;
    background?: string;
}

 // define default durations (I just picked random ones)
// put it in types.ts so it's visible in the documentation later
 export const defaultDurations = {
     icon: 'duration-400',
     background: 'duration-500',
     onOffSlots: 'duration-400'
 };

switch.svelte

<script lang="ts">
    // ... props
    export let transitionDurations: SwitchTransitionDurations = {};

    // overwrite default durations
    $: durations = { ...defaultDurations, transitionDurations };
</script>

<!-- html stuff as example -->
<button class="{durations.background} other-classes" />

Also, if the users only use icons instead of text there would need to be a label for the switch for it to be accessible. In the old one I had an ariaLabel attribute that could be hidden, which still made it available for screen readers. What do you think about adding this?

The headless example we have for the switch can be left as it currently is, this way there's two variations. For the radio group I also made two variations of the component, but I don't think for the switch it would make sense since they are too similar (?). Otherwise the old switch would also have to be update to look good for RTL.

And last point, we should still have the different available sizes as before I think. Let me know if you have thoughts on these.

Mohamed-Kaizen commented 9 months ago

I will updated it and will come to discuss it in detail.

bennymi commented 9 months ago

@Mohamed-Kaizen awesome, yeah it will be easier to discuss in the PR :)