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

[Bug]: Toggle ignores labelA and labelB and uses labelText if hideLabel is specified #12896

Closed
2 tasks done
kglickman opened this issue Dec 21, 2022 · 5 comments · Fixed by #12974
Closed
2 tasks done

Comments

@kglickman
Copy link

Package

@carbon/react

Browser

No response

Package version

1.19.0

React version

16.14.0

Description

We don't want to use the Carbon label for the toggle because we are providing our own label that includes a tooltip. We have set hideLabel to true but now it is using our labelText next to the toggle instead of using the labelA and labelB.

Suggested Severity

Severity 1 = Must be fixed ASAP. The response must be swift. Someone from the team must drop all current work and be immediately reassigned to address the issue.

Reproduction/example

https://stackblitz.com/edit/github-shdz33

Steps to reproduce

Set the values of labelA, labelB, labelText, and set hideLabel to true.

Code of Conduct

@tw15egan
Copy link
Collaborator

Refs #12094
Refs #12810

cc @janhassel

@janhassel
Copy link
Member

Hi @kglickman!

Would passing your custom label as props.labelText work for your use case?

<Toggle
  id="toggle-1"
  hideLabel
  labelText={(
    <>
      Label
      <Tooltip label="Helper text">
        <button>
          <Information />
        </button>
      </Tooltip>
    </>
  )}
/>

@kglickman
Copy link
Author

It does work for the use case I mentioned in the defect. However, we also have use cases where the label must be displayed outside the component.

@janhassel
Copy link
Member

Hmm, for now you could manually set .cds--toggle__text to display: none (note that props.labelText is still required and its content will still be read by screen readers).

@tw15egan What do you think? Moving the top label to the side is a common use case seen throughout a lot of products which is why using props.hideLabel this way seemed elegant. Adding a way to completely hide the label with props.hideLabel would probably count as a breaking change as its behavior is also stated in the v11 migration docs. I can see the use case of having an external label associated though as long as the two elements (toggle and external label) are linked properly via aria-labelledby or a <label for="<id>">.

Maybe one of the following options could work?

  1. If one passes a string prefixed with # to props.labelText, the component uses this as the id for aria-labelledby instead of writing it out.
  2. If props.hideLabel === true and aria-labelledby was passed, emit the aria-labelledby onto the <button> and don't render props.labelText.

The second one seems more predictable imo.

@tw15egan
Copy link
Collaborator

I agree. The second option seems like a more predictable solution for this scenario

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants