Wildcard: Fix disabled target tooltip (fix disabled button state) (#44384)

* Add alternative disabled state to the button UI

* Simplify tooltip target disabled case

* Fix button styles

* Fix integration tests

* Update unit test snapshots

* Add custom matchers utility for aria disabled state

* Fix button as checkbox disabled styles

* Fix focus for the button checkbox

* Fix button styles
This commit is contained in:
Vova Kulikov 2022-11-17 21:29:45 -03:00 committed by GitHub
parent 3741c43f53
commit 13d8693ea9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
31 changed files with 208 additions and 163 deletions

View File

@ -3,6 +3,7 @@ import userEvent from '@testing-library/user-event'
import { act } from 'react-dom/test-utils'
import sinon from 'sinon'
import { assertAriaDisabled, assertAriaEnabled } from '@sourcegraph/shared/dev/aria-asserts'
import { NOOP_TELEMETRY_SERVICE } from '@sourcegraph/shared/src/telemetry/telemetryService'
import { MockIntersectionObserver } from '@sourcegraph/shared/src/testing/MockIntersectionObserver'
import {
@ -68,19 +69,19 @@ describe('SearchContextDropdown', () => {
it('should be enabled if query is empty', () => {
render(<SearchContextDropdown {...defaultProps} />)
expect(screen.getByTestId('dropdown-toggle')).toBeEnabled()
assertAriaEnabled(screen.getByTestId('dropdown-toggle'))
expect(screen.getByTestId('dropdown-toggle')).toHaveAttribute('data-test-tooltip-content', '')
})
it('should be enabled if query does not contain context filter', () => {
render(<SearchContextDropdown {...defaultProps} query="test (repo:foo or repo:python)" />)
expect(screen.getByTestId('dropdown-toggle')).toBeEnabled()
assertAriaEnabled(screen.getByTestId('dropdown-toggle'))
expect(screen.getByTestId('dropdown-toggle')).toHaveAttribute('data-test-tooltip-content', '')
})
it('should be disabled if query contains context filter', () => {
render(<SearchContextDropdown {...defaultProps} query="test (context:foo or repo:python)" />)
expect(screen.getByTestId('dropdown-toggle')).toBeDisabled()
assertAriaDisabled(screen.getByTestId('dropdown-toggle'))
expect(screen.getByTestId('dropdown-toggle')).toHaveAttribute(
'data-test-tooltip-content',
'Overridden by query'

View File

@ -2,6 +2,7 @@ import { screen, within } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import sinon from 'sinon'
import { assertAriaDisabled, assertAriaEnabled } from '@sourcegraph/shared/dev/aria-asserts'
import { Progress } from '@sourcegraph/shared/src/search/stream'
import { renderWithBrandedContext } from '@sourcegraph/shared/src/testing'
@ -113,7 +114,7 @@ describe('StreamingProgressSkippedPopover', () => {
const form = screen.getByTestId('popover-form')
const searchAgainButton = within(form).getByRole('button')
expect(searchAgainButton).toBeInTheDocument()
expect(searchAgainButton).toBeDisabled()
assertAriaDisabled(searchAgainButton)
})
it('should enable Search Again button if at least one item is checked', () => {
@ -164,7 +165,7 @@ describe('StreamingProgressSkippedPopover', () => {
const form = screen.getByTestId('popover-form')
const searchAgainButton = within(form).getByRole('button')
expect(searchAgainButton).toBeInTheDocument()
expect(searchAgainButton).toBeEnabled()
assertAriaEnabled(searchAgainButton)
})
it('should disable Search Again button if unchecking all items', () => {
@ -214,10 +215,10 @@ describe('StreamingProgressSkippedPopover', () => {
const form = screen.getByTestId('popover-form')
const searchAgainButton = within(form).getByRole('button')
expect(searchAgainButton).toBeEnabled()
assertAriaEnabled(searchAgainButton)
userEvent.click(checkboxes[1])
expect(searchAgainButton).toBeDisabled()
assertAriaDisabled(searchAgainButton)
})
it('should call onSearchAgain with selected items when button is clicked', () => {

View File

@ -16,6 +16,7 @@ exports[`StreamingProgressSkippedPopover should render correctly 1`] = `
class="pt-2 w-100 streamingSkippedItem streamingSkippedItemWarn"
>
<button
aria-disabled="false"
aria-expanded="true"
class="btn btnDanger btnOutline button p-2 w-100 bg-transparent border-0"
role="button"
@ -78,6 +79,7 @@ exports[`StreamingProgressSkippedPopover should render correctly 1`] = `
class="pt-2 w-100 streamingSkippedItem streamingSkippedItemWarn"
>
<button
aria-disabled="false"
aria-expanded="false"
class="btn btnDanger btnOutline button p-2 w-100 bg-transparent border-0"
role="button"
@ -140,6 +142,7 @@ exports[`StreamingProgressSkippedPopover should render correctly 1`] = `
class="pt-2 w-100 streamingSkippedItem"
>
<button
aria-disabled="false"
aria-expanded="false"
class="btn btnPrimary btnOutline button p-2 w-100 bg-transparent border-0"
role="button"
@ -202,6 +205,7 @@ exports[`StreamingProgressSkippedPopover should render correctly 1`] = `
class="pt-2 w-100 streamingSkippedItem"
>
<button
aria-disabled="false"
aria-expanded="false"
class="btn btnPrimary btnOutline button p-2 w-100 bg-transparent border-0"
role="button"
@ -264,6 +268,7 @@ exports[`StreamingProgressSkippedPopover should render correctly 1`] = `
class="pt-2 w-100 streamingSkippedItem"
>
<button
aria-disabled="false"
aria-expanded="false"
class="btn btnPrimary btnOutline button p-2 w-100 bg-transparent border-0"
role="button"
@ -482,9 +487,9 @@ exports[`StreamingProgressSkippedPopover should render correctly 1`] = `
</div>
</div>
<button
aria-disabled="true"
class="btn btnPrimary mt-2"
data-testid="skipped-popover-form-submit-btn"
disabled=""
type="submit"
>
<svg

View File

@ -0,0 +1,22 @@
// Since jest doesn't provide native matcher to check aria state of the element
// see https://github.com/testing-library/jest-dom/issues/144 for more details.
// We have to use our in-house assert utility for this. We can't use custom jest
// matcher `.toBeAriaDisabled` due to problems with TS global types problems,
// see https://github.com/sourcegraph/sourcegraph/pull/44461
/**
* Checks element aria-disabled and disabled state. Fails if element is disabled.
*/
export function assertAriaEnabled(element: HTMLElement): void {
expect(element.getAttribute('aria-disabled') !== 'true' && element.getAttribute('disabled') === null).toBe(true)
}
/**
* Checks element aria-disabled and disabled state. Fails if element is active.
*/
export function assertAriaDisabled(element: HTMLElement): void {
const nativeDisabled = (element as HTMLButtonElement).disabled
const ariaDisabled = element.getAttribute('aria-disabled') === 'true'
expect(ariaDisabled || nativeDisabled).toBe(true)
}

View File

@ -3,6 +3,7 @@ import userEvent from '@testing-library/user-event'
import * as H from 'history'
import { NEVER } from 'rxjs'
import { assertAriaEnabled } from '../../dev/aria-asserts'
import { createBarrier } from '../api/integration-test/testHelpers'
import { NOOP_TELEMETRY_SERVICE } from '../telemetry/telemetryService'
import { renderWithBrandedContext } from '../testing'
@ -185,8 +186,8 @@ describe('ActionItem', () => {
// to result in the setState call.)
userEvent.click(screen.getByRole('button'))
// we should wait for the button to be enabled again after got errors. Otherwise it will be flaky
await waitFor(() => expect(screen.getByLabelText('d')).toBeEnabled())
// we should wait for the button to be enabled again after got errors. Otherwise, it will be flaky
await waitFor(() => assertAriaEnabled(screen.getByLabelText('d')))
expect(asFragment()).toMatchSnapshot()
})

View File

@ -195,81 +195,57 @@ exports[`ActionItem run command with error with showInlineError 1`] = `
exports[`ActionItem run command with showLoadingSpinnerDuringExecution 1`] = `
<DocumentFragment>
<span
class="tooltipWrapper"
<button
aria-disabled="true"
aria-label="d"
class="test-action-item actionItemLoading"
type="button"
>
<img
alt="d"
src="u"
/>
g: t
<div
class="tooltipTriggerContainer"
class="loader"
data-testid="action-item-spinner"
>
<div
class="tooltipTriggerDisabledOverlay"
tabindex="0"
aria-label="Loading"
aria-live="polite"
class="mdi-icon loadingSpinner"
role="img"
/>
<button
aria-label="d"
class="test-action-item actionItemLoading"
disabled=""
type="button"
>
<img
alt="d"
src="u"
/>
g: t
<div
class="loader"
data-testid="action-item-spinner"
>
<div
aria-label="Loading"
aria-live="polite"
class="mdi-icon loadingSpinner"
role="img"
/>
</div>
</button>
</div>
</span>
</button>
</DocumentFragment>
`;
exports[`ActionItem run command with showLoadingSpinnerDuringExecution 2`] = `
<DocumentFragment>
<span
class="tooltipWrapper"
<button
aria-disabled="true"
aria-label="d"
class="test-action-item actionItemLoading"
type="button"
>
<img
alt="d"
src="u"
/>
g: t
<div
class="tooltipTriggerContainer"
class="loader"
data-testid="action-item-spinner"
>
<div
class="tooltipTriggerDisabledOverlay"
tabindex="0"
aria-label="Loading"
aria-live="polite"
class="mdi-icon loadingSpinner"
role="img"
/>
<button
aria-label="d"
class="test-action-item actionItemLoading"
disabled=""
type="button"
>
<img
alt="d"
src="u"
/>
g: t
<div
class="loader"
data-testid="action-item-spinner"
>
<div
aria-label="Loading"
aria-live="polite"
class="mdi-icon loadingSpinner"
role="img"
/>
</div>
</button>
</div>
</span>
</button>
</DocumentFragment>
`;

View File

@ -121,6 +121,7 @@ exports[`SignInPage renders sign in page (cloud) 1`] = `
class="form-group"
>
<button
aria-disabled="false"
class="btn btnPrimary btnBlock"
type="submit"
>
@ -297,6 +298,7 @@ exports[`SignInPage renders sign in page (server) 1`] = `
class="form-group"
>
<button
aria-disabled="false"
class="btn btnPrimary btnBlock"
type="submit"
>
@ -473,6 +475,7 @@ exports[`SignInPage with Sourcegraph auth provider renders page with 2 providers
class="form-group"
>
<button
aria-disabled="false"
class="btn btnPrimary btnBlock"
type="submit"
>
@ -649,6 +652,7 @@ exports[`SignInPage with Sourcegraph auth provider renders page with 3 providers
class="form-group"
>
<button
aria-disabled="false"
class="btn btnPrimary btnBlock"
type="submit"
>
@ -835,6 +839,7 @@ exports[`SignInPage with Sourcegraph auth provider renders page with 3 providers
class="form-group"
>
<button
aria-disabled="false"
class="btn btnPrimary btnBlock"
type="submit"
>

View File

@ -272,8 +272,8 @@ exports[`SignUpPage renders sign up page (server) 1`] = `
class="form-group mb-0"
>
<button
aria-disabled="true"
class="btn btnPrimary btnBlock d-flex justify-content-center align-items-center"
disabled=""
type="submit"
>
Register

View File

@ -38,6 +38,7 @@ exports[`ExternalServiceForm create GitHub 1`] = `
DynamicallyImportedMonacoSettingsEditor
</div>
<button
aria-disabled="false"
class="btn btnPrimary mb-3 test-add-external-service-button"
type="submit"
>
@ -85,6 +86,7 @@ exports[`ExternalServiceForm edit GitHub 1`] = `
DynamicallyImportedMonacoSettingsEditor
</div>
<button
aria-disabled="false"
class="btn btnPrimary mb-3 test-add-external-service-button"
type="submit"
>
@ -133,8 +135,8 @@ exports[`ExternalServiceForm edit GitHub, loading 1`] = `
DynamicallyImportedMonacoSettingsEditor
</div>
<button
aria-disabled="true"
class="btn btnPrimary mb-3 test-add-external-service-button"
disabled=""
type="submit"
>
<div

View File

@ -4,6 +4,7 @@ import * as H from 'history'
import { NEVER, of } from 'rxjs'
import sinon from 'sinon'
import { assertAriaDisabled } from '@sourcegraph/shared/dev/aria-asserts'
import { renderWithBrandedContext } from '@sourcegraph/shared/src/testing'
import { MockedTestProvider } from '@sourcegraph/shared/src/testing/apollo'
@ -106,6 +107,6 @@ describe('CreateCodeMonitorPage', () => {
</MockedTestProvider>
)
const actionButton = screen.getByTestId('form-action-toggle-email')
expect(actionButton).toBeDisabled()
assertAriaDisabled(actionButton)
})
})

View File

@ -4,6 +4,7 @@ import * as H from 'history'
import { NEVER, of } from 'rxjs'
import sinon from 'sinon'
import { assertAriaDisabled, assertAriaEnabled } from '@sourcegraph/shared/dev/aria-asserts'
import {
MonitorEditInput,
MonitorEditTriggerInput,
@ -157,11 +158,11 @@ describe('ManageCodeMonitorPage', () => {
</MockedTestProvider>
)
const submitButton = screen.getByTestId('submit-monitor')
expect(submitButton).toBeDisabled()
assertAriaDisabled(submitButton)
userEvent.type(screen.getByTestId('name-input'), 'Test code monitor updated')
expect(submitButton).toBeEnabled()
assertAriaEnabled(submitButton)
})
test('Cancelling after changes have been made shows confirmation prompt', () => {

View File

@ -2,6 +2,7 @@ import { fireEvent, getByRole, screen } from '@testing-library/react'
import { createMemoryHistory, createLocation } from 'history'
import { NEVER } from 'rxjs'
import { assertAriaDisabled } from '@sourcegraph/shared/dev/aria-asserts'
import { renderWithBrandedContext } from '@sourcegraph/shared/src/testing'
import { MockedTestProvider } from '@sourcegraph/shared/src/testing/apollo'
@ -51,7 +52,7 @@ describe('CodeMonitorForm', () => {
fireEvent.click(getByTestId('form-action-toggle-email'))
fireEvent.click(getByTestId('delete-action-email'))
expect(getByTestId('submit-monitor')).toBeDisabled()
assertAriaDisabled(getByTestId('submit-monitor'))
})
test('Submit button enabled if one action is present', () => {

View File

@ -87,9 +87,9 @@ exports[`FormActionArea Error is shown if code monitor has empty description 1`]
class="flex mt-1"
>
<button
aria-disabled="true"
class="btn btnSecondary btnSm mr-2"
data-testid="send-test-email"
disabled=""
type="button"
>
Send test email
@ -132,6 +132,7 @@ exports[`FormActionArea Error is shown if code monitor has empty description 1`]
class="actionButtonRow"
>
<button
aria-disabled="false"
class="btn btnSecondary test-submit-action-email"
data-testid="submit-action-email"
type="button"

View File

@ -2,6 +2,8 @@ import { render } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import sinon from 'sinon'
import { assertAriaDisabled } from '@sourcegraph/shared/dev/aria-asserts'
import { ActionEditor, ActionEditorProps } from './ActionEditor'
describe('ActionEditor', () => {
@ -84,7 +86,7 @@ describe('ActionEditor', () => {
userEvent.click(getByTestId('form-action-toggle-email'))
expect(queryByTestId('delete-action-email')).not.toBeInTheDocument()
expect(getByTestId('submit-action-email')).toBeDisabled()
assertAriaDisabled(getByTestId('submit-action-email'))
})
test('toggle disable when collapsed', () => {

View File

@ -3,6 +3,7 @@ import { render } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import sinon from 'sinon'
import { assertAriaDisabled, assertAriaEnabled } from '@sourcegraph/shared/dev/aria-asserts'
import { MockedTestProvider, waitForNextApolloResponse } from '@sourcegraph/shared/src/testing/apollo'
import { MonitorEmailPriority, SendTestEmailResult, SendTestEmailVariables } from '../../../../graphql-operations'
@ -190,7 +191,7 @@ describe('EmailAction', () => {
)
userEvent.click(getByTestId('form-action-toggle-email'))
expect(getByTestId('send-test-email')).toBeDisabled()
assertAriaDisabled(getByTestId('send-test-email'))
})
test('send test email, success', async () => {
@ -217,7 +218,7 @@ describe('EmailAction', () => {
await waitForNextApolloResponse()
expect(getByTestId('send-test-email')).toHaveTextContent('Test email sent!')
expect(getByTestId('send-test-email')).toBeDisabled()
assertAriaDisabled(getByTestId('send-test-email'))
expect(queryByTestId('send-test-email-again')).toBeInTheDocument()
expect(queryByTestId('test-email-error')).not.toBeInTheDocument()
@ -247,7 +248,7 @@ describe('EmailAction', () => {
expect(getByTestId('send-test-email')).toHaveTextContent('Send test email')
expect(getByTestId('send-test-email')).toBeEnabled()
assertAriaEnabled(getByTestId('send-test-email'))
expect(queryByTestId('send-test-email-again')).not.toBeInTheDocument()
expect(queryByTestId('test-email-error')).toBeInTheDocument()

View File

@ -3,6 +3,7 @@ import { render } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import sinon from 'sinon'
import { assertAriaDisabled, assertAriaEnabled } from '@sourcegraph/shared/dev/aria-asserts'
import { MockedTestProvider, waitForNextApolloResponse } from '@sourcegraph/shared/src/testing/apollo'
import { SendTestSlackWebhookResult, SendTestSlackWebhookVariables } from '../../../../graphql-operations'
@ -32,10 +33,10 @@ describe('SlackWebhookAction', () => {
userEvent.click(getByTestId('form-action-toggle-slack-webhook'))
expect(getByTestId('submit-action-slack-webhook')).toBeDisabled()
assertAriaDisabled(getByTestId('submit-action-slack-webhook'))
userEvent.type(getByTestId('slack-webhook-url'), SLACK_URL)
expect(getByTestId('submit-action-slack-webhook')).toBeEnabled()
assertAriaEnabled(getByTestId('submit-action-slack-webhook'))
userEvent.click(getByTestId('include-results-toggle-slack-webhook'))
@ -69,13 +70,13 @@ describe('SlackWebhookAction', () => {
)
userEvent.click(getByTestId('form-action-toggle-slack-webhook'))
expect(getByTestId('submit-action-slack-webhook')).toBeEnabled()
assertAriaEnabled(getByTestId('submit-action-slack-webhook'))
userEvent.clear(getByTestId('slack-webhook-url'))
expect(getByTestId('submit-action-slack-webhook')).toBeDisabled()
assertAriaDisabled(getByTestId('submit-action-slack-webhook'))
userEvent.type(getByTestId('slack-webhook-url'), SLACK_URL)
expect(getByTestId('submit-action-slack-webhook')).toBeEnabled()
assertAriaEnabled(getByTestId('submit-action-slack-webhook'))
userEvent.click(getByTestId('submit-action-slack-webhook'))
@ -217,7 +218,7 @@ describe('SlackWebhookAction', () => {
)
userEvent.click(getByTestId('form-action-toggle-slack-webhook'))
expect(getByTestId('send-test-slack-webhook')).toBeDisabled()
assertAriaDisabled(getByTestId('send-test-slack-webhook'))
})
test('disabled if no monitor name set', () => {
@ -228,7 +229,7 @@ describe('SlackWebhookAction', () => {
)
userEvent.click(getByTestId('form-action-toggle-slack-webhook'))
expect(getByTestId('send-test-slack-webhook')).toBeDisabled()
assertAriaDisabled(getByTestId('send-test-slack-webhook'))
})
test('send test message, success', async () => {
@ -255,7 +256,7 @@ describe('SlackWebhookAction', () => {
await waitForNextApolloResponse()
expect(getByTestId('send-test-slack-webhook')).toHaveTextContent('Test message sent!')
expect(getByTestId('send-test-slack-webhook')).toBeDisabled()
assertAriaDisabled(getByTestId('send-test-slack-webhook'))
expect(queryByTestId('send-test-slack-webhook')).toBeInTheDocument()
expect(queryByTestId('test-email-slack-webhook')).not.toBeInTheDocument()
@ -285,7 +286,7 @@ describe('SlackWebhookAction', () => {
expect(getByTestId('send-test-slack-webhook')).toHaveTextContent('Send test message')
expect(getByTestId('send-test-slack-webhook')).toBeEnabled()
assertAriaEnabled(getByTestId('send-test-slack-webhook'))
expect(queryByTestId('send-test-slack-webhook-again')).not.toBeInTheDocument()
expect(queryByTestId('test-slack-webhook-error')).toBeInTheDocument()

View File

@ -3,6 +3,7 @@ import { render } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import sinon from 'sinon'
import { assertAriaDisabled, assertAriaEnabled } from '@sourcegraph/shared/dev/aria-asserts'
import { MockedTestProvider, waitForNextApolloResponse } from '@sourcegraph/shared/src/testing/apollo'
import { SendTestWebhookResult, SendTestWebhookVariables } from '../../../../graphql-operations'
@ -30,10 +31,10 @@ describe('WebhookAction', () => {
userEvent.click(getByTestId('form-action-toggle-webhook'))
expect(getByTestId('submit-action-webhook')).toBeDisabled()
assertAriaDisabled(getByTestId('submit-action-webhook'))
userEvent.type(getByTestId('webhook-url'), 'https://example.com')
expect(getByTestId('submit-action-webhook')).toBeEnabled()
assertAriaEnabled(getByTestId('submit-action-webhook'))
userEvent.click(getByTestId('include-results-toggle-webhook'))
@ -67,13 +68,13 @@ describe('WebhookAction', () => {
)
userEvent.click(getByTestId('form-action-toggle-webhook'))
expect(getByTestId('submit-action-webhook')).toBeEnabled()
assertAriaEnabled(getByTestId('submit-action-webhook'))
userEvent.clear(getByTestId('webhook-url'))
expect(getByTestId('submit-action-webhook')).toBeDisabled()
assertAriaDisabled(getByTestId('submit-action-webhook'))
userEvent.type(getByTestId('webhook-url'), 'https://example2.com')
expect(getByTestId('submit-action-webhook')).toBeEnabled()
assertAriaEnabled(getByTestId('submit-action-webhook'))
userEvent.click(getByTestId('submit-action-webhook'))
@ -215,7 +216,7 @@ describe('WebhookAction', () => {
)
userEvent.click(getByTestId('form-action-toggle-webhook'))
expect(getByTestId('send-test-webhook')).toBeDisabled()
assertAriaDisabled(getByTestId('send-test-webhook'))
})
test('disabled if no monitor name set', () => {
@ -226,7 +227,7 @@ describe('WebhookAction', () => {
)
userEvent.click(getByTestId('form-action-toggle-webhook'))
expect(getByTestId('send-test-webhook')).toBeDisabled()
assertAriaDisabled(getByTestId('send-test-webhook'))
})
test('send test message, success', async () => {
@ -253,7 +254,7 @@ describe('WebhookAction', () => {
await waitForNextApolloResponse()
expect(getByTestId('send-test-webhook')).toHaveTextContent('Test call completed!')
expect(getByTestId('send-test-webhook')).toBeDisabled()
assertAriaDisabled(getByTestId('send-test-webhook'))
expect(queryByTestId('send-test-webhook')).toBeInTheDocument()
expect(queryByTestId('test-email-webhook')).not.toBeInTheDocument()
@ -283,7 +284,7 @@ describe('WebhookAction', () => {
expect(getByTestId('send-test-webhook')).toHaveTextContent('Call webhook with test payload')
expect(getByTestId('send-test-webhook')).toBeEnabled()
assertAriaEnabled(getByTestId('send-test-webhook'))
expect(queryByTestId('send-test-webhook-again')).not.toBeInTheDocument()
expect(queryByTestId('test-webhook-error')).toBeInTheDocument()

View File

@ -146,6 +146,7 @@ exports[`SiteAdminGenerateProductLicenseForSubscriptionForm renders 1`] = `
</small>
</div>
<button
aria-disabled="false"
class="btn btnPrimary"
type="submit"
>

View File

@ -83,6 +83,7 @@ exports[`SiteAdminProductLicenseNode active 1`] = `
class="input-group-append"
>
<button
aria-disabled="false"
aria-label="Copy"
class="btn btnSecondary"
type="button"
@ -187,6 +188,7 @@ exports[`SiteAdminProductLicenseNode inactive 1`] = `
class="input-group-append"
>
<button
aria-disabled="false"
aria-label="Copy"
class="btn btnSecondary"
type="button"

View File

@ -37,6 +37,7 @@ exports[`SiteAdminProductSubscriptionPage renders 1`] = `
class="mb-3"
>
<button
aria-disabled="false"
class="btn btnDanger"
type="button"
>
@ -238,6 +239,7 @@ exports[`SiteAdminProductSubscriptionPage renders 1`] = `
class="input-group-append"
>
<button
aria-disabled="false"
aria-label="Copy"
class="btn btnSecondary"
type="button"

View File

@ -10,6 +10,7 @@ exports[`StatusBar renders correctly 1`] = `
class="d-flex align-items-center px-2 items"
>
<a
aria-disabled="false"
class="h-100 d-flex align-items-center px-1 item text-decoration-none itemNoop"
href=""
role="button"
@ -22,6 +23,7 @@ exports[`StatusBar renders correctly 1`] = `
</small>
</a>
<a
aria-disabled="false"
class="h-100 d-flex align-items-center px-1 item text-decoration-none itemNoop"
href=""
role="button"

View File

@ -10,7 +10,7 @@ import { afterEachSaveScreenshotIfFailed } from '@sourcegraph/shared/src/testing
import { WebIntegrationTestContext, createWebIntegrationTestContext } from './context'
import { commonWebGraphQlResults } from './graphQlResults'
import { siteID, siteGQLID } from './jscontext'
import { createEditorAPI, percySnapshotWithVariants } from './utils'
import { createEditorAPI, isElementDisabled, percySnapshotWithVariants } from './utils'
describe('Code monitoring', () => {
let driver: Driver
@ -128,9 +128,7 @@ describe('Code monitoring', () => {
await driver.page.waitForSelector('.test-action-button-email')
assert.strictEqual(
await driver.page.evaluate(
() => document.querySelector<HTMLButtonElement>('.test-action-button-email')!.disabled
),
await isElementDisabled(driver, '.test-action-button-email'),
true,
'Expected action button to be disabled'
)
@ -157,9 +155,7 @@ describe('Code monitoring', () => {
await driver.page.waitForSelector('.test-action-button-email')
assert.strictEqual(
await driver.page.evaluate(
() => document.querySelector<HTMLButtonElement>('.test-action-button-email')!.disabled
),
await isElementDisabled(driver, '.test-action-button-email'),
true,
'Expected action button to be disabled'
)
@ -177,9 +173,7 @@ describe('Code monitoring', () => {
await driver.page.waitForSelector('.test-action-button-email')
assert.strictEqual(
await driver.page.evaluate(
() => document.querySelector<HTMLButtonElement>('.test-action-button-email')!.disabled
),
await isElementDisabled(driver, '.test-action-button-email'),
false,
'Expected action button to be enabled'
)
@ -195,9 +189,7 @@ describe('Code monitoring', () => {
await driver.page.waitForSelector('.test-submit-monitor')
assert.strictEqual(
await driver.page.evaluate(
() => document.querySelector<HTMLButtonElement>('.test-submit-monitor')!.disabled
),
await isElementDisabled(driver, '.test-submit-monitor'),
true,
'Expected submit monitor button to be disabled'
)
@ -219,9 +211,7 @@ describe('Code monitoring', () => {
await driver.page.click('.test-submit-action-email')
assert.strictEqual(
await driver.page.evaluate(
() => document.querySelector<HTMLButtonElement>('.test-submit-monitor')!.disabled
),
await isElementDisabled(driver, '.test-submit-monitor'),
false,
'Expected submit monitor button to be enabled'
)

View File

@ -8,7 +8,7 @@ import { retry } from '@sourcegraph/shared/src/testing/utils'
import { createWebIntegrationTestContext, WebIntegrationTestContext } from './context'
import { commonWebGraphQlResults } from './graphQlResults'
import { createEditorAPI, percySnapshotWithVariants } from './utils'
import { createEditorAPI, isElementDisabled, percySnapshotWithVariants } from './utils'
describe('Settings', () => {
let driver: Driver
@ -95,9 +95,7 @@ describe('Settings', () => {
await driver.page.waitForSelector('.test-save-toolbar-save')
assert.strictEqual(
await driver.page.evaluate(
() => document.querySelector<HTMLButtonElement>('.test-save-toolbar-save')?.disabled
),
await isElementDisabled(driver, '.test-save-toolbar-save'),
true,
'Expected save button to be disabled'
)

View File

@ -347,3 +347,13 @@ export const withSearchQueryInput = (callback: (editorName: Editor) => void): vo
callback(editor)
}
}
export const isElementDisabled = (driver: Driver, query: string): Promise<boolean> =>
driver.page.evaluate((query: string) => {
const element = document.querySelector<HTMLButtonElement>(query)
const disabledAttribute = element!.disabled
const ariaDisabled = element!.getAttribute('aria-disabled')
return disabledAttribute || ariaDisabled === 'true'
}, query)

View File

@ -134,8 +134,8 @@ exports[`SiteInitPage normal 1`] = `
class="form-group mb-0"
>
<button
aria-disabled="true"
class="btn btnPrimary btnBlock d-flex justify-content-center align-items-center"
disabled=""
type="submit"
>
Create admin account & continue

View File

@ -29,10 +29,9 @@
border-radius: var(--border-radius);
padding: 0.375rem 0.75rem;
&:disabled {
&:disabled,
&[aria-disabled='true'] {
cursor: not-allowed;
// This is needed to properly support Tooltip behavior on disabled buttons
pointer-events: none;
}
&:hover {
@ -41,12 +40,35 @@
}
}
// Handle a special case for Button as="input" type="checkbox | radio"
// since button doesn't use disabled attribute we enforce/mimic the native
// disabled control styles.
input.btn {
&:focus {
outline: 2px solid var(--primary-2);
outline-offset: 2px;
}
&[aria-disabled='true'] {
cursor: not-allowed;
:global(.theme-light) & {
filter: brightness(0.9);
}
:global(.theme-dark) & {
filter: brightness(0.5);
}
}
}
.btn-link {
color: var(--link-color);
background-color: var(--link-1);
&:global(.disabled),
&:disabled {
&:disabled,
&[aria-disabled='true'] {
:global(.theme-light) & {
opacity: 0.2;
}
@ -56,7 +78,7 @@
}
}
&:not(:disabled):not(:global(.disabled)) {
&:not(:disabled):not(:global(.disabled)):not([aria-disabled='true']) {
&:hover:not(:global(.focus)):not(:focus),
&:active,
&:global(.active) {
@ -115,16 +137,17 @@
border: none;
cursor: pointer;
&:focus-visible:not(:disabled):not(:global(.disabled)) {
&:focus-visible:not(:disabled):not(:global(.disabled)):not([aria-disabled='true']) {
box-shadow: 0 0 0 2px var(--primary-2);
}
&:hover:not(:disabled):not(:global(.disabled)) {
&:hover:not(:disabled):not(:global(.disabled)):not([aria-disabled='true']) {
color: var(--body-color);
}
// Add color for disabled state of button icon
&:disabled {
&:disabled,
&[aria-disabled='true'] {
:global(.theme-light) & {
color: var(--btn-icon-disabled-light-color);
}
@ -154,7 +177,8 @@
background-color: var(--btn-base-color);
&:global(.disabled),
&:disabled {
&:disabled,
&[aria-disabled='true'] {
opacity: 1;
background-color: var(--btn-light-color-variant);
border-color: var(--btn-light-color-variant);
@ -167,7 +191,7 @@
}
}
&:not(:disabled):not(:global(.disabled)) {
&:not(:disabled):not(:global(.disabled)):not([aria-disabled='true']) {
&:hover:not(:global(.focus)):not(:focus),
&:active,
&:global(.active) {
@ -208,14 +232,15 @@
background-color: transparent;
&:global(.disabled),
&:disabled {
&:disabled,
&[aria-disabled='true'] {
--btn-light-disabled-text-color: var(--text-disabled);
--btn-dark-disabled-text-color: var(--text-disabled);
background-color: transparent;
}
&:not(:disabled):not(:global(.disabled)) {
&:not(:disabled):not(:global(.disabled)):not([aria-disabled='true']) {
// Set inlined icons to match outline border
// except [data-caret] attribute (which can be used for button carret
svg:not([data-caret]) {
@ -327,11 +352,12 @@
// btn-secondary needs specific styles to ensure good contrast
.btn-secondary {
&:global(.disabled),
&:disabled {
&:disabled,
&[aria-disabled='true'] {
border-color: var(--input-disabled-bg);
}
&:not(:disabled):not(:global(.disabled)) {
&:not(:disabled):not(:global(.disabled)):not([aria-disabled='true']) {
&:hover:not(:global(.focus)):not(:focus),
&:active,
&:global(.active) {

View File

@ -1,4 +1,4 @@
import React from 'react'
import { MouseEvent, ButtonHTMLAttributes, forwardRef } from 'react'
import classNames from 'classnames'
@ -8,7 +8,7 @@ import { ForwardReferenceComponent } from '../../types'
import { BUTTON_VARIANTS, BUTTON_SIZES, BUTTON_DISPLAY } from './constants'
import { getButtonClassName } from './utils'
export interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> {
export interface ButtonProps extends ButtonHTMLAttributes<HTMLButtonElement> {
/**
* The variant style of the button. Defaults to `primary`
*/
@ -44,7 +44,7 @@ export interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElemen
* - Avoid using button styling for links where possible. Buttons should typically trigger an action, links should navigate to places.
*/
export const Button = React.forwardRef(
export const Button = forwardRef(
(
{
children,
@ -57,6 +57,7 @@ export const Button = React.forwardRef(
className,
disabled,
display,
onClick,
...attributes
},
reference
@ -65,12 +66,19 @@ export const Button = React.forwardRef(
const brandedButtonClassname = getButtonClassName({ variant, outline, display, size })
const handleClick = (event: MouseEvent<HTMLButtonElement>): void => {
if (!disabled) {
onClick?.(event)
}
}
return (
<Component
ref={reference}
className={classNames(isBranded && brandedButtonClassname, className)}
type={type}
disabled={disabled}
aria-disabled={disabled}
className={classNames(isBranded && brandedButtonClassname, className)}
onClick={handleClick}
{...attributes}
>
{children}

View File

@ -3,8 +3,8 @@
exports[`<ButtonLink /> renders correctly \`disabled\` 1`] = `
<DocumentFragment>
<a
aria-disabled="true"
class="anchorLink btn btnSecondary btnLg disabled"
disabled=""
href=""
role="button"
tabindex="-1"

View File

@ -2,6 +2,8 @@ import { cleanup, fireEvent, render, screen } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import sinon from 'sinon'
import { assertAriaDisabled, assertAriaEnabled } from '@sourcegraph/shared/dev/aria-asserts'
import { Button } from '../../Button'
import { PopoverTrigger } from '../../Popover'
@ -39,7 +41,7 @@ describe('FeedbackPrompt', () => {
target: { value: sampleFeedback.feedback },
})
expect(screen.getByText('Send')).toBeEnabled()
assertAriaEnabled(screen.getByText('Send'))
userEvent.click(screen.getByText('Send'))
}
@ -49,11 +51,11 @@ describe('FeedbackPrompt', () => {
})
test('should enable/disable submit button correctly', () => {
expect(screen.getByText('Send')).toBeDisabled()
assertAriaDisabled(screen.getByText('Send'))
userEvent.type(screen.getByLabelText('Send feedback to Sourcegraph'), sampleFeedback.feedback)
expect(screen.getByText('Send')).toBeEnabled()
assertAriaEnabled(screen.getByText('Send'))
})
test('should render submit success correctly', async () => {

View File

@ -70,8 +70,8 @@ exports[`FeedbackPrompt should render correctly 1`] = `
You're not signed in. Please tell us how to contact you if you want a reply.
</p>
<button
aria-disabled="true"
class="button"
disabled=""
role="menuitem"
type="submit"
>
@ -176,6 +176,7 @@ exports[`FeedbackPrompt should render submit error correctly 1`] = `
</span>
</div>
<button
aria-disabled="false"
class="button"
role="menuitem"
type="submit"

View File

@ -195,28 +195,9 @@ const TooltipTarget = forwardRef<any, TooltipTargetProps>(function TooltipTarget
const { 'aria-describedby': ariaDescribedby, children } = props
const mergedRef = useMergeRefs([forwardedRef, (children as any).ref])
let trigger: React.ReactElement
// Disabled buttons come through with a disabled prop and must be wrapped with a
// span in order for the Tooltip to work properly
// Reference: https://www.radix-ui.com/docs/primitives/components/tooltip#displaying-a-tooltip-from-a-disabled-button
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
if (children.props?.disabled) {
trigger = (
<span className={styles.tooltipWrapper}>
<div className={styles.tooltipTriggerContainer}>
{/* eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex */}
<div className={styles.tooltipTriggerDisabledOverlay} tabIndex={0} />
{children}
</div>
</span>
)
} else {
trigger = children
}
if (React.isValidElement(trigger)) {
return React.cloneElement(trigger as ReactElement, {
if (React.isValidElement(children)) {
return React.cloneElement(children as ReactElement, {
'aria-describedby': ariaDescribedby,
ref: mergedRef,
})