From 9f4aa472cfc100761ee52b4525cd16d237ad3728 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Fri, 26 Nov 2021 16:38:48 +0100 Subject: [PATCH] Select: Fix input cursor position so that it is at the start for single value selects (#41693) * Select: Fix input cursor position so that it is at the start for single value selects * Fixing e2e tests * Fixes cursor issue * Fixing e2e tests * e2e fix * Select: ensure input always overlays singleValue, update pa11y config Co-authored-by: Ashley Harrison --- .pa11yci-pr.conf.js | 4 +- .pa11yci.conf.js | 4 +- .../dashboard-time-zone.spec.ts | 2 +- .../variables/new-query-variable.ts | 6 +-- e2e/various-suite/exemplars.spec.ts | 2 +- e2e/various-suite/query-editor.spec.ts | 2 +- .../trace-view-scrolling.spec.ts | 4 +- .../grafana-e2e/src/flows/importDashboard.ts | 2 +- .../src/components/Select/InputControl.tsx | 3 -- .../src/components/Select/SelectBase.tsx | 2 +- .../src/components/Select/SingleValue.tsx | 47 +++++++++---------- .../src/components/Select/getSelectStyles.ts | 4 +- .../components/Select/resetSelectStyles.ts | 13 ++++- 13 files changed, 53 insertions(+), 42 deletions(-) diff --git a/.pa11yci-pr.conf.js b/.pa11yci-pr.conf.js index 308d95b6785..dc01c7857c2 100644 --- a/.pa11yci-pr.conf.js +++ b/.pa11yci-pr.conf.js @@ -6,7 +6,9 @@ var config = { chromeLaunchConfig: { args: ['--no-sandbox'], }, - hideElements: '#updateVersion', + // see https://github.com/grafana/grafana/pull/41693#issuecomment-979921463 for context + // on why we're ignoring singleValue/react-select-*-placeholder elements + hideElements: '#updateVersion, [class*="-singleValue"], [id^="react-select-"][id$="-placeholder"]', }, urls: [ diff --git a/.pa11yci.conf.js b/.pa11yci.conf.js index 0d7a9c11361..b2fbf4274f7 100644 --- a/.pa11yci.conf.js +++ b/.pa11yci.conf.js @@ -6,7 +6,9 @@ var config = { chromeLaunchConfig: { args: ['--no-sandbox'], }, - hideElements: '#updateVersion', + // see https://github.com/grafana/grafana/pull/41693#issuecomment-979921463 for context + // on why we're ignoring singleValue/react-select-*-placeholder elements + hideElements: '#updateVersion, [class*="-singleValue"], [id^="react-select-"][id$="-placeholder"]', }, urls: [ diff --git a/e2e/dashboards-suite/dashboard-time-zone.spec.ts b/e2e/dashboards-suite/dashboard-time-zone.spec.ts index cfa9ffce794..e2e8a69432e 100644 --- a/e2e/dashboards-suite/dashboard-time-zone.spec.ts +++ b/e2e/dashboards-suite/dashboard-time-zone.spec.ts @@ -53,7 +53,7 @@ e2e.scenario({ e2e.components.TimeZonePicker.containerV2() .should('be.visible') .within(() => { - e2e.components.Select.singleValue().should('be.visible').should('have.text', 'Coordinated Universal Time'); + e2e.components.Select.singleValue().should('have.text', 'Coordinated Universal Time'); e2e.components.Select.input().should('be.visible').click(); }); diff --git a/e2e/smoke-tests-suite/variables/new-query-variable.ts b/e2e/smoke-tests-suite/variables/new-query-variable.ts index f8a15aad4d6..4efd16bd1bf 100644 --- a/e2e/smoke-tests-suite/variables/new-query-variable.ts +++ b/e2e/smoke-tests-suite/variables/new-query-variable.ts @@ -18,7 +18,7 @@ describe('Variables - Add variable', () => { e2e.pages.Dashboard.Settings.Variables.Edit.General.generalTypeSelect() .should('be.visible') .within((select) => { - e2e.components.Select.singleValue().should('be.visible').should('have.text', 'Query'); + e2e.components.Select.singleValue().should('have.text', 'Query'); }); e2e.pages.Dashboard.Settings.Variables.Edit.General.generalLabelInput() .should('be.visible') @@ -82,7 +82,7 @@ describe('Variables - Add variable', () => { e2e().get('#Description').should('be.visible').clear().type('a description'); - e2e.components.DataSourcePicker.inputV2().should('be.visible').type('gdev-testdata{enter}'); + e2e.components.DataSourcePicker.container().should('be.visible').type('gdev-testdata{enter}'); e2e.pages.Dashboard.Settings.Variables.Edit.QueryVariable.queryOptionsQueryInput() .should('be.visible') @@ -132,7 +132,7 @@ describe('Variables - Add variable', () => { e2e().get('#Description').should('be.visible').clear().type('a description'); - e2e.components.DataSourcePicker.inputV2().should('be.visible').type('gdev-testdata{enter}'); + e2e.components.DataSourcePicker.container().type('gdev-testdata{enter}'); e2e.pages.Dashboard.Settings.Variables.Edit.QueryVariable.queryOptionsQueryInput() .should('be.visible') diff --git a/e2e/various-suite/exemplars.spec.ts b/e2e/various-suite/exemplars.spec.ts index 2fb174b96e2..5a89c3528cd 100644 --- a/e2e/various-suite/exemplars.spec.ts +++ b/e2e/various-suite/exemplars.spec.ts @@ -44,7 +44,7 @@ describe('Exemplars', () => { e2e.pages.Explore.visit(); - e2e.components.DataSourcePicker.input().should('be.visible').click(); + e2e.components.DataSourcePicker.container().should('be.visible').click(); e2e().contains(dataSourceName).scrollIntoView().should('be.visible').click(); // we need to wait for the query-field being lazy-loaded, in two steps: diff --git a/e2e/various-suite/query-editor.spec.ts b/e2e/various-suite/query-editor.spec.ts index e99275ce19b..90afade11d3 100644 --- a/e2e/various-suite/query-editor.spec.ts +++ b/e2e/various-suite/query-editor.spec.ts @@ -8,7 +8,7 @@ e2e.scenario({ skipScenario: false, scenario: () => { e2e.pages.Explore.visit(); - e2e.components.DataSourcePicker.inputV2().should('be.visible').click(); + e2e.components.DataSourcePicker.container().should('be.visible').click(); cy.contains('gdev-prometheus').scrollIntoView().should('be.visible').click(); const queryText = 'http_requests_total'; diff --git a/e2e/various-suite/trace-view-scrolling.spec.ts b/e2e/various-suite/trace-view-scrolling.spec.ts index 44ca2892ec0..69a8bb191f8 100644 --- a/e2e/various-suite/trace-view-scrolling.spec.ts +++ b/e2e/various-suite/trace-view-scrolling.spec.ts @@ -11,9 +11,7 @@ describe('Trace view', () => { e2e.pages.Explore.visit(); - e2e.components.DataSourcePicker.inputV2().should('be.visible').click(); - - e2e().contains('gdev-jaeger').scrollIntoView().should('be.visible').click(); + e2e.components.DataSourcePicker.container().should('be.visible').type('gdev-jaeger{enter}'); e2e.components.QueryField.container().should('be.visible').type('long-trace'); diff --git a/packages/grafana-e2e/src/flows/importDashboard.ts b/packages/grafana-e2e/src/flows/importDashboard.ts index d4eadd1c412..84631082655 100644 --- a/packages/grafana-e2e/src/flows/importDashboard.ts +++ b/packages/grafana-e2e/src/flows/importDashboard.ts @@ -50,7 +50,7 @@ export const importDashboard = (dashboardToImport: Dashboard, queryTimeout?: num e2e.components.Panels.Panel.title(panel.title).should('be.visible').click(); e2e.components.Panels.Panel.headerItems('Inspect').should('be.visible').click(); e2e.components.Tab.title('JSON').should('be.visible').click(); - e2e.components.PanelInspector.Json.content().should('be.visible').contains('Panel JSON').click(); + e2e.components.PanelInspector.Json.content().should('be.visible').contains('Panel JSON').click({ force: true }); e2e.components.Select.option().should('be.visible').contains('Data').click(); // ensures that panel has loaded without knowingly hitting an error diff --git a/packages/grafana-ui/src/components/Select/InputControl.tsx b/packages/grafana-ui/src/components/Select/InputControl.tsx index 0121f601eab..d06a268ebc7 100644 --- a/packages/grafana-ui/src/components/Select/InputControl.tsx +++ b/packages/grafana-ui/src/components/Select/InputControl.tsx @@ -25,15 +25,12 @@ const getInputControlStyles = stylesFactory( css` width: 100%; max-width: 100%; - display: flex; flex-direction: row; align-items: center; flex-wrap: wrap; justify-content: space-between; - padding-right: 0; - position: relative; box-sizing: border-box; `, diff --git a/packages/grafana-ui/src/components/Select/SelectBase.tsx b/packages/grafana-ui/src/components/Select/SelectBase.tsx index 18c5d853fe6..f1c4b0df0a6 100644 --- a/packages/grafana-ui/src/components/Select/SelectBase.tsx +++ b/packages/grafana-ui/src/components/Select/SelectBase.tsx @@ -260,7 +260,7 @@ export function SelectBase({ css` display: inline-block; color: ${theme.colors.text.disabled}; - + grid-area: 1 / 1 / 2 / 3; box-sizing: border-box; line-height: 1; white-space: nowrap; diff --git a/packages/grafana-ui/src/components/Select/SingleValue.tsx b/packages/grafana-ui/src/components/Select/SingleValue.tsx index 64a03ca9096..8ae91d4ab4c 100644 --- a/packages/grafana-ui/src/components/Select/SingleValue.tsx +++ b/packages/grafana-ui/src/components/Select/SingleValue.tsx @@ -18,8 +18,9 @@ const getStyles = (theme: GrafanaTheme2) => { text-overflow: ellipsis; box-sizing: border-box; max-width: 100%; + grid-area: 1 / 1 / 2 / 3; `; - const container = css` + const spinnerWrapper = css` width: 16px; height: 16px; display: inline-block; @@ -29,7 +30,7 @@ const getStyles = (theme: GrafanaTheme2) => { overflow: hidden; `; - const item = css` + const spinnerIcon = css` width: 100%; height: 100%; position: absolute; @@ -39,7 +40,7 @@ const getStyles = (theme: GrafanaTheme2) => { color: ${tinycolor(theme.colors.text.disabled).setAlpha(0.64).toString()}; `; - return { singleValue, container, item, disabled }; + return { singleValue, spinnerWrapper, spinnerIcon, disabled }; }; type StylesType = ReturnType; @@ -52,36 +53,34 @@ export const SingleValue = (props: Props) => { const loading = useDelayedSwitch(data.loading || false, { delay: 250, duration: 750 }); return ( - -
- {data.imgUrl ? ( - - ) : ( - -
- -
- - )} - {!data.hideText && children} -
+ + {data.imgUrl ? ( + + ) : ( + +
+ +
+ + )} + {!data.hideText && children} ); }; const FadeWithImage = (props: { loading: boolean; imgUrl: string; styles: StylesType; alt?: string }) => { return ( -
+
- + - {props.alt} + {props.alt}
); diff --git a/packages/grafana-ui/src/components/Select/getSelectStyles.ts b/packages/grafana-ui/src/components/Select/getSelectStyles.ts index 9d025aaad83..e3e2c7a4606 100644 --- a/packages/grafana-ui/src/components/Select/getSelectStyles.ts +++ b/packages/grafana-ui/src/components/Select/getSelectStyles.ts @@ -66,6 +66,7 @@ export const getSelectStyles = stylesFactory((theme: GrafanaTheme2) => { singleValue: css` label: grafana-select-single-value; color: ${theme.components.input.text}; + grid-area: 1 / 1 / 2 / 3; white-space: nowrap; overflow: hidden; text-overflow: ellipsis; @@ -75,7 +76,7 @@ export const getSelectStyles = stylesFactory((theme: GrafanaTheme2) => { valueContainer: css` label: grafana-select-value-container; align-items: center; - display: flex; + display: grid; position: relative; box-sizing: border-box; flex: 1 1 0%; @@ -85,6 +86,7 @@ export const getSelectStyles = stylesFactory((theme: GrafanaTheme2) => { valueContainerMulti: css` label: grafana-select-value-container-multi; flex-wrap: wrap; + display: flex; `, loadingMessage: css` label: grafana-select-loading-message; diff --git a/packages/grafana-ui/src/components/Select/resetSelectStyles.ts b/packages/grafana-ui/src/components/Select/resetSelectStyles.ts index a980741c17c..4b31dacd8cc 100644 --- a/packages/grafana-ui/src/components/Select/resetSelectStyles.ts +++ b/packages/grafana-ui/src/components/Select/resetSelectStyles.ts @@ -1,3 +1,5 @@ +import { CSSObjectWithLabel } from 'react-select'; + export default function resetSelectStyles() { return { clearIndicator: () => ({}), @@ -8,7 +10,16 @@ export default function resetSelectStyles() { groupHeading: () => ({}), indicatorsContainer: () => ({}), indicatorSeparator: () => ({}), - input: () => ({}), + input: function (originalStyles: CSSObjectWithLabel) { + return { + ...originalStyles, + color: 'inherit', + margin: 0, + padding: 0, + // Set an explicit z-index here to ensure this element always overlays the singleValue + zIndex: 1, + }; + }, loadingIndicator: () => ({}), loadingMessage: () => ({}), menu: () => ({}),