From 0603ae6ca98077e090ef8c6b64a558a23b7698b8 Mon Sep 17 00:00:00 2001 From: Phillip Burch Date: Wed, 4 Dec 2019 17:44:51 -0600 Subject: [PATCH 01/66] Don't allow duplicate saved views with the same name (#52040) * Don't allow duplicate saved views with the same name * Change logic to make it a little easier to reason about * Change error names --- .../components/saved_views/create_modal.tsx | 5 +- .../saved_views/toolbar_control.tsx | 48 ++++++++++++++----- .../public/hooks/use_create_saved_object.tsx | 3 ++ .../public/hooks/use_find_saved_object.tsx | 9 ++++ .../infra/public/hooks/use_saved_view.ts | 32 ++++++++++--- 5 files changed, 77 insertions(+), 20 deletions(-) diff --git a/x-pack/legacy/plugins/infra/public/components/saved_views/create_modal.tsx b/x-pack/legacy/plugins/infra/public/components/saved_views/create_modal.tsx index 2fc96fbd22a9e..8df479f36e2f9 100644 --- a/x-pack/legacy/plugins/infra/public/components/saved_views/create_modal.tsx +++ b/x-pack/legacy/plugins/infra/public/components/saved_views/create_modal.tsx @@ -23,11 +23,12 @@ import { } from '@elastic/eui'; interface Props { + isInvalid: boolean; close(): void; save(name: string, shouldIncludeTime: boolean): void; } -export const SavedViewCreateModal = ({ close, save }: Props) => { +export const SavedViewCreateModal = ({ close, save, isInvalid }: Props) => { const [viewName, setViewName] = useState(''); const [includeTime, setIncludeTime] = useState(false); const onCheckChange = useCallback(e => setIncludeTime(e.target.checked), []); @@ -35,7 +36,6 @@ export const SavedViewCreateModal = ({ close, save }: Props) => { const saveView = useCallback(() => { save(viewName, includeTime); - close(); }, [viewName, includeTime]); return ( @@ -52,6 +52,7 @@ export const SavedViewCreateModal = ({ close, save }: Props) => { (props: Props) { find, errorOnFind, errorOnCreate, + createdId, } = useSavedView(props.defaultViewState, props.viewType); const [modalOpen, setModalOpen] = useState(false); + const [isInvalid, setIsInvalid] = useState(false); const [createModalOpen, setCreateModalOpen] = useState(false); - const openSaveModal = useCallback(() => setCreateModalOpen(true), []); - const closeCreateModal = useCallback(() => setCreateModalOpen(false), []); + const openSaveModal = useCallback(() => { + setIsInvalid(false); + setCreateModalOpen(true); + }, []); const closeModal = useCallback(() => setModalOpen(false), []); + const closeCreateModal = useCallback(() => setCreateModalOpen(false), []); const loadViews = useCallback(() => { find(); setModalOpen(true); @@ -50,6 +55,19 @@ export function SavedViewsToolbarControls(props: Props) { [props.viewState, saveView] ); + useEffect(() => { + if (errorOnCreate) { + setIsInvalid(true); + } + }, [errorOnCreate]); + + useEffect(() => { + if (createdId !== undefined) { + // INFO: Close the modal after the view is created. + closeCreateModal(); + } + }, [createdId, closeCreateModal]); + useEffect(() => { if (deletedId !== undefined) { // INFO: Refresh view list after an item is deleted @@ -59,9 +77,9 @@ export function SavedViewsToolbarControls(props: Props) { useEffect(() => { if (errorOnCreate) { - toastNotifications.addWarning(getErrorToast('create')!); + toastNotifications.addWarning(getErrorToast('create', errorOnCreate)!); } else if (errorOnFind) { - toastNotifications.addWarning(getErrorToast('find')!); + toastNotifications.addWarning(getErrorToast('find', errorOnFind)!); } }, [errorOnCreate, errorOnFind]); @@ -82,7 +100,9 @@ export function SavedViewsToolbarControls(props: Props) { - {createModalOpen && } + {createModalOpen && ( + + )} {modalOpen && ( loading={loading} @@ -96,18 +116,22 @@ export function SavedViewsToolbarControls(props: Props) { ); } -const getErrorToast = (type: 'create' | 'find') => { +const getErrorToast = (type: 'create' | 'find', msg?: string) => { if (type === 'create') { return { - title: i18n.translate('xpack.infra.savedView.errorOnCreate.title', { - defaultMessage: `An error occured saving view.`, - }), + title: + msg || + i18n.translate('xpack.infra.savedView.errorOnCreate.title', { + defaultMessage: `An error occured saving view.`, + }), }; } else if (type === 'find') { return { - title: i18n.translate('xpack.infra.savedView.findError.title', { - defaultMessage: `An error occurred while loading views.`, - }), + title: + msg || + i18n.translate('xpack.infra.savedView.findError.title', { + defaultMessage: `An error occurred while loading views.`, + }), }; } }; diff --git a/x-pack/legacy/plugins/infra/public/hooks/use_create_saved_object.tsx b/x-pack/legacy/plugins/infra/public/hooks/use_create_saved_object.tsx index 944249033e10b..80811a6d6c7bf 100644 --- a/x-pack/legacy/plugins/infra/public/hooks/use_create_saved_object.tsx +++ b/x-pack/legacy/plugins/infra/public/hooks/use_create_saved_object.tsx @@ -12,6 +12,7 @@ import { SavedObjectAttributes } from 'src/core/server'; export const useCreateSavedObject = (type: string) => { const [data, setData] = useState | null>(null); + const [createdId, setCreatedId] = useState(null); const [error, setError] = useState(null); const [loading, setLoading] = useState(false); @@ -21,6 +22,7 @@ export const useCreateSavedObject = (type: string) => { const save = async () => { try { const d = await npStart.core.savedObjects.client.create(type, attributes, options); + setCreatedId(d.id); setError(null); setData(d); setLoading(false); @@ -39,5 +41,6 @@ export const useCreateSavedObject = (type: string) => { loading, error, create, + createdId, }; }; diff --git a/x-pack/legacy/plugins/infra/public/hooks/use_find_saved_object.tsx b/x-pack/legacy/plugins/infra/public/hooks/use_find_saved_object.tsx index 27e01b23e6b4c..949a2344418e9 100644 --- a/x-pack/legacy/plugins/infra/public/hooks/use_find_saved_object.tsx +++ b/x-pack/legacy/plugins/infra/public/hooks/use_find_saved_object.tsx @@ -37,7 +37,16 @@ export const useFindSavedObject = { + const objects = await npStart.core.savedObjects.client.find({ + type, + }); + + return objects.savedObjects.filter(o => o.attributes.name === name).length > 0; + }; + return { + hasView, data, loading, error, diff --git a/x-pack/legacy/plugins/infra/public/hooks/use_saved_view.ts b/x-pack/legacy/plugins/infra/public/hooks/use_saved_view.ts index f3d4833a5a755..8db0ed28d9b21 100644 --- a/x-pack/legacy/plugins/infra/public/hooks/use_saved_view.ts +++ b/x-pack/legacy/plugins/infra/public/hooks/use_saved_view.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { useCallback, useMemo } from 'react'; +import { useCallback, useMemo, useState, useEffect } from 'react'; import { i18n } from '@kbn/i18n'; import { useFindSavedObject } from './use_find_saved_object'; import { useCreateSavedObject } from './use_create_saved_object'; @@ -16,18 +16,37 @@ export type SavedView = ViewState & { isDefault?: boolean; }; -export type SavedViewSavedObject = ViewState & { +export type SavedViewSavedObject = ViewState & { name: string; }; export const useSavedView = (defaultViewState: ViewState, viewType: string) => { - const { data, loading, find, error: errorOnFind } = useFindSavedObject< + const { data, loading, find, error: errorOnFind, hasView } = useFindSavedObject< SavedViewSavedObject >(viewType); - const { create, error: errorOnCreate } = useCreateSavedObject(viewType); + const { create, error: errorOnCreate, createdId } = useCreateSavedObject(viewType); const { deleteObject, deletedId } = useDeleteSavedObject(viewType); const deleteView = useCallback((id: string) => deleteObject(id), []); - const saveView = useCallback((d: { [p: string]: any }) => create(d), []); + const [createError, setCreateError] = useState(null); + + useEffect(() => setCreateError(createError), [errorOnCreate, setCreateError]); + + const saveView = useCallback((d: { [p: string]: any }) => { + const doSave = async () => { + const exists = await hasView(d.name); + if (exists) { + setCreateError( + i18n.translate('xpack.infra.savedView.errorOnCreate.duplicateViewName', { + defaultMessage: `A view with that name already exists.`, + }) + ); + return; + } + create(d); + }; + setCreateError(null); + doSave(); + }, []); const savedObjects = data ? data.savedObjects : []; const views = useMemo(() => { @@ -61,8 +80,9 @@ export const useSavedView = (defaultViewState: ViewState, viewType: s saveView, loading, deletedId, + createdId, errorOnFind, - errorOnCreate, + errorOnCreate: createError, deleteView, find, }; From e8f3fa91d9eca3d0e14465d6c1a5848b0abbefc3 Mon Sep 17 00:00:00 2001 From: Michail Yasonik Date: Wed, 4 Dec 2019 19:11:16 -0600 Subject: [PATCH 02/66] Updating accessibility guide and contributing readmes (#52038) * updating accessibility guide and contributing readmes * updating dev docs --- CONTRIBUTING.md | 2 +- .../development-functional-tests.asciidoc | 3 +- style_guides/accessibility_guide.md | 243 ++---------------- x-pack/README.md | 2 +- 4 files changed, 19 insertions(+), 231 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 53e44fbede724..17d2ca85a54ba 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -405,7 +405,7 @@ The following table outlines possible test file locations and how to invoke them | Jest | `src/**/*.test.js`
`src/**/*.test.ts` | `node scripts/jest -t regexp [test path]` | | Jest (integration) | `**/integration_tests/**/*.test.js` | `node scripts/jest_integration -t regexp [test path]` | | Mocha | `src/**/__tests__/**/*.js`
`!src/**/public/__tests__/*.js`
`packages/kbn-datemath/test/**/*.js`
`packages/kbn-dev-utils/src/**/__tests__/**/*.js`
`tasks/**/__tests__/**/*.js` | `node scripts/mocha --grep=regexp [test path]` | -| Functional | `test/*integration/**/config.js`
`test/*functional/**/config.js` | `node scripts/functional_tests_server --config test/[directory]/config.js`
`node scripts/functional_test_runner --config test/[directory]/config.js --grep=regexp` | +| Functional | `test/*integration/**/config.js`
`test/*functional/**/config.js`
`test/accessibility/config.js` | `node scripts/functional_tests_server --config test/[directory]/config.js`
`node scripts/functional_test_runner --config test/[directory]/config.js --grep=regexp` | | Karma | `src/**/public/__tests__/*.js` | `npm run test:dev` | For X-Pack tests located in `x-pack/` see [X-Pack Testing](x-pack/README.md#testing) diff --git a/docs/developer/core/development-functional-tests.asciidoc b/docs/developer/core/development-functional-tests.asciidoc index 6d2c5a72f0532..350a3c2a997cf 100644 --- a/docs/developer/core/development-functional-tests.asciidoc +++ b/docs/developer/core/development-functional-tests.asciidoc @@ -98,6 +98,7 @@ When run without any arguments the `FunctionalTestRunner` automatically loads th * `--config test/functional/config.js` starts Elasticsearch and Kibana servers with the WebDriver tests configured to run in Chrome. * `--config test/functional/config.firefox.js` starts Elasticsearch and Kibana servers with the WebDriver tests configured to run in Firefox. * `--config test/api_integration/config.js` starts Elasticsearch and Kibana servers with the api integration tests configuration. +* `--config test/accessibility/config.ts` starts Elasticsearch and Kibana servers with the WebDriver tests configured to run an accessibility audit using https://www.deque.com/axe/[axe]. There are also command line flags for `--bail` and `--grep`, which behave just like their mocha counterparts. For instance, use `--grep=foo` to run only tests that match a regular expression. @@ -362,7 +363,7 @@ Full list of services that are used in functional tests can be found here: {blob ** Source: {blob}test/functional/services/remote/remote.ts[test/functional/services/remote/remote.ts] ** Instance of https://seleniumhq.github.io/selenium/docs/api/javascript/module/selenium-webdriver/index_exports_WebDriver.html[WebDriver] class ** Responsible for all communication with the browser -** To perform browser actions, use `remote` service +** To perform browser actions, use `remote` service ** For searching and manipulating with DOM elements, use `testSubjects` and `find` services ** See the https://seleniumhq.github.io/selenium/docs/api/javascript/[selenium-webdriver docs] for the full API. diff --git a/style_guides/accessibility_guide.md b/style_guides/accessibility_guide.md index 28fb69ec38185..4827d93991510 100644 --- a/style_guides/accessibility_guide.md +++ b/style_guides/accessibility_guide.md @@ -1,59 +1,25 @@ # Accessibility (A11Y) Guide -This document provides some technical guidelines how to prevent several common -accessibility issues. +[EUI's accessibility guidelines](https://elastic.github.io/eui/#/guidelines/accessibility) should be your first stop for all things. -## Naming elements +## Automated accessibility testing -### `aria-label` and `aria-labelledby` +To run the tests locally: -Every element on a page will have a name, that is read out to an assistive technology -like a screen reader. This will for most elements be the content of the element. -For form elements it will be the content of the associated `