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

[EuiButtonGroup] Automatically apply title when in iconOnly mode #5180

Closed
cchaos opened this issue Sep 14, 2021 · 5 comments
Closed

[EuiButtonGroup] Automatically apply title when in iconOnly mode #5180

cchaos opened this issue Sep 14, 2021 · 5 comments

Comments

@cchaos
Copy link
Contributor

cchaos commented Sep 14, 2021

We still require a label to be supplied in iconOnly mode which makes it work for screen readers, but we didn't build anything in for mouse users to get hint text. Consumers can currently work around this by manually adding the title prop to the items config, but I think we should also just do this automatically since we already have a label.

Screen Shot 2021-09-14 at 12 44 10 PM

@lejara
Copy link
Contributor

lejara commented Sep 15, 2021

Hello!
New to this repo, is it okay if i can work on this?

@cchaos
Copy link
Contributor Author

cchaos commented Sep 15, 2021

@lejara Go for it!

@lejara
Copy link
Contributor

lejara commented Sep 16, 2021

After adding the title prop to button_group_button.tsx

Screenshot from 2021-09-16 17-08-25

The snapshot tests in button_group.test.tsx are now failing.
Is it okay if i can update the failed snapshots to account for the title prop?

Failed test output:

 FAIL  src/components/button/button_group/button_group.test.tsx
  ● EuiButtonGroup › button props › isIconOnly › is rendered for single

    expect(received).toMatchSnapshot()

    Snapshot name: `EuiButtonGroup button props isIconOnly is rendered for single 1`

    - Snapshot
    + Received

    @@ -12,10 +12,11 @@
          <label
            aria-label="aria-label"
            class="euiButtonGroupButton euiButtonGroupButton--text euiButtonGroupButton--small euiButtonGroupButton-isIconOnly testClass1 testClass2"
            data-test-subj="test subject string"
            for="generated-id"
    +       title="Option one"
          >
            <span
              class="euiButtonContent euiButton__content"
            >
              <span
    @@ -38,10 +39,11 @@
            </span>
          </label>
          <label
            class="euiButtonGroupButton euiButtonGroupButton--text euiButtonGroupButton--small euiButtonGroupButton-isIconOnly"
            for="generated-id"
    +       title="Option two"
          >
            <span
              class="euiButtonContent euiButton__content"
            >
              <span
    @@ -67,10 +69,11 @@
            aria-pressed="false"
            class="euiButtonGroupButton euiButtonGroupButton--text euiButtonGroupButton--small euiButtonGroupButton-isDisabled euiButtonGroupButton-isIconOnly"
            data-test-subj="button02"
            disabled=""
            id="generated-id"
    +       title="Option three"
            type="submit"
          >
            <span
              class="euiButtonContent euiButton__content"
            >

      140 |         );
      141 | 
    > 142 |         expect(component).toMatchSnapshot();
          |                           ^
      143 |       });
      144 |       it('is rendered for multi', () => {
      145 |         const component = render(

      at Object.<anonymous> (src/components/button/button_group/button_group.test.tsx:142:27)

@cchaos
Copy link
Contributor Author

cchaos commented Sep 16, 2021

@lejara Yep! Just run yarn test-unit -u to update those.

@cchaos
Copy link
Contributor Author

cchaos commented Sep 23, 2021

Closed via #5199

@cchaos cchaos closed this as completed Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants