appuniversum / ember-appuniversum

Ember addon wrapping the appuniversum components.
https://appuniversum.github.io/ember-appuniversum
MIT License
14 stars 11 forks source link

`AuLoader` updates #464

Closed Windvis closed 9 months ago

Windvis commented 9 months ago

This updates the AuLoader component so it matches the Webuniversum <vl-ui-loader> component.

Notable changes:

Closes #456

Migration guide

The existing options that aren't supported by the <vl-ui-loader> component are now deprecated.

No arguments / default loading message

If you were using the AuLoader component without any arguments the component would use a (visually hidden) default message. You can achieve a similar result by passing the previous default message as the block content.

{{! Before }}
<AuLoader />

{{! After }}
<AuLoader @hideMessage={{true}}>Aan het laden</AuLoader>

@message

Use the block content of the component to pass the loading message. If you want this loading message to only be visible for screenreaders you can combine this with the @hideMessage argument.

Before:

{{! Before }}
<AuLoader @message="Only visible for screenreaders" />

{{! After }}
<AuLoader @hideMessage={{true}}>Only visible for screenreaders</AuLoader>

@disableMessage

Disabling the message is not recommended for accessibility reasons. This option was originally added because there was no way to display a visible loading text with the previous implementation. Apps could then disable the internal hidden one, and add a visual loading message themselves. This is now no longer needed since the block content will be used as the visual message by default.

{{! Before }}
<AuLoader @disableMessage={{true}} />
<p>Some loading message</p>

{{! After }}
<AuLoader>Some loading message</AuLoader>

If you have a valid reason for not showing any sort of loading message, please open an issue to request this.

@padding

The padding argument was used to align the loader with other whitespace on the page, since the loader is now centered this shouldn't be needed anymore.

It's still possible to add whitespace by using the spacing util classes.

Component alignment

The new loader defaults to a centered position to match the Webuniversum version. There are use-cases where this can look a bit strange so for those you can set the @centered argument to false and add extra classes to align the component however you want.

[!NOTE]
@centered was added in the 3.2.0 release

Windvis commented 9 months ago

We also use the AuLoader in the AuButton component, and I think we don't want to output any loading text there, since the button already does that, but the current PR doesn't support that (in a non-deprecated way).

We could keep the @disableMessage but I'm not sure if we should. I think we want to push the version with a loading message for accessibilty reasons, but maybe there are valid cases where we only want the loading animation. We could go for a private argument that we only use in Appuniversum itself maybe. πŸ€”.

Another option could be to display the inline version inside the button instead, maybe? It will look a bit different from the current version though.

Edit: Hmm, I just looked at the Webuniversum version, and they use a custom animation there, not the vl-ui-loader inside the button. Maybe we should do a similar thing here?

@brenner-company Any insights?

brenner-company commented 9 months ago

I didn't have the time today to give this a look. I'll pick this up on wednesday if that's ok?

brenner-company commented 9 months ago

I think both the 'private argument' & 'custom animation (within the button)' are good options. I'm not a fan of the 'inline version' solution because of, as you said, the visual difference.

Could it also be an option to extract the <div class="au-c-loader__animation" aria-hidden="true"></div> as a private component (somethin along addon/private/components/loader-animation) and use that within the actual loader & button component? Maybe I'm overthinking it πŸ˜…

Windvis commented 9 months ago

I think both the 'private argument' & 'custom animation (within the button)' are good options. I'm not a fan of the 'inline version' solution because of, as you said, the visual difference.

βœ…

Could it also be an option to extract the

as a private component (somethin along addon/private/components/loader-animation) and use that within the actual loader & button component? Maybe I'm overthinking it πŸ˜…

Yea, that's indeed an option πŸ‘

The main thing I'm not sure about is, do we want to prevent apps from using the AuLoader component without a loading message (so not a non-visual one either). With the current component that's possible. With the new one that won't be. I don't think it's needed, but I might be missing use-cases πŸ˜….