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

Scale Link: Attribute "aria-disabled" stays "true" when link is not disabled anymore #1806

Closed
georgwittberger-telekom-mms opened this issue May 8, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@georgwittberger-telekom-mms

Scale Version

3.0.0-beta.131

Framework and version

React 18.2.0 with @telekom/scale-components-react

Current Behavior

Once Scale Link has been disabled the attribute aria-disabled="true" persists on the inner <a> element even when link is rendered without disabled later.

Expected Behavior

When Scale Link is rendered without disabled attribute or with disabled="false" the attribute aria-disabled="true" is removed from the inner <a> element or its value is set to "false".

Code Reproduction

Initial HTML:

<scale-link disabled href="#top">A link, disabled</scale-link>

Then re-render scale-link element without attribute disabled or with disabled="false".

Desktop:

  • OS: Windows
  • Browser: Chrome/Edge
  • Version: 113

Additional context

The problem seems to be on this line:

'aria-disabled': this.disabled ? 'true' : false,

It seems like setting the value false inside this props object does not remove the respective attribute once it has been rendered. Maybe consider changing this to:

'aria-disabled': this.disabled ? 'true' : 'false', // notice 'false' is a string now
@georgwittberger-telekom-mms georgwittberger-telekom-mms added the bug Something isn't working label May 8, 2023
@felix-ico
Copy link
Collaborator

@georgwittberger-telekom-mms thank you for opening the issue, this should be fixed in the next release (beta.133)

@georgwittberger-telekom-mms
Copy link
Author

The attribute value changes as expected now but when the link is not disabled the attribute value is undefined now:

<a part="anchor" aria-disabled="undefined">...</a>

This is not spec-compliant: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-disabled

The value of aria-disabled should be false when the element is not disabled.

@felix-ico
Copy link
Collaborator

Hi @georgwittberger-telekom-mms, are you testing on the latest version? I am not seeing the issue on the latest version here https://telekom.github.io/scale/?path=/docs/components-link--standard

Screenshot 2023-06-12 at 17 22 25

@georgwittberger-telekom-mms
Copy link
Author

This is not reproducible in Storybook because it renders a fresh version of the component every time you change props.

In our project - currently on 3.0.0-beta.134 - it happens when a <scale-link> has the attribute disabled="true" set and then the attribute is removed from that existing element. The component updates as expected but it seems like when attributes are changed the value of the disabled attribute is not coerced to a boolean value. Instead it seems like the string value is simply passed on to aria-disabled.

@felix-ico
Copy link
Collaborator

@georgwittberger-telekom-mms thanks for the feedback, I've made a simple https://codesandbox.io/s/scale-components-plain-html-template-forked-vzv595?file=/index.html sandbox to test this but still could not reproduce the bug. Are you using some framework to build your app ? (I also tested this on react and it seemed to work as expected)

@georgwittberger-telekom-mms
Copy link
Author

Hi @felix-ico , we are using React and the bindings you provide for it. I was able to find out how to reproduce the "undefined" issue.

It occurs when you always pass the disabled prop to ScaleLink but in case you want to link to be enabled you explicitly pass undefined instead of false or omitting the prop entirely.

import { ScaleLink } from "@telekom/scale-components-react";
import React, { useState } from "react";

const Test: React.FC = () => {
  const [disabled, setDisabled] = useState(false);
  return (
    <>
      <button onClick={() => setDisabled(prevDisabled => !prevDisabled)}>
        Toggle Disabled
      </button>
      <ScaleLink href="#" disabled={disabled || undefined}>
        Test Link
      </ScaleLink>
    </>
  );
};

This is what happens once you swap states:

chrome_kx7pr2ZUnQ

I suppose we should fix this on our side and make sure we always pass a boolean value to disabled. Nevertheless, other projects could be affected by this accidental "misuse" as well. I think it could be solved with my suggested solution from above:

'aria-disabled': this.disabled ? 'true' : 'false', // notice 'false' is a string now

or

'aria-disabled': `${!!this.disabled}`, // coerce it to boolean explicitly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants