Remove unused vars by thomasneirynck · Pull Request #8558 · elastic/kibana

thomasneirynck · 2016-10-05T15:43:00Z

fix #8533. cleanup test code.

kobelb · 2016-10-05T15:58:12Z

test/support/page_objects/visualize_page.js

If this parameter isn't being used anymore, the functional tests appear to be still providing it, should we consider removing it there?

https://github.com/elastic/kibana/blob/master/test/functional/apps/visualize/_line_chart.js#L65
https://github.com/elastic/kibana/blob/master/test/functional/apps/visualize/_line_chart.js#L95

thomasneirynck · 2016-10-05T22:04:21Z

@kobelb you're right. I removed those too. These were introduced at 89aff02.

became obsolete at some point, though can't quite pinpoint where.

kobelb · 2016-10-05T22:05:05Z

LGTM

LeeDr · 2016-10-05T22:24:38Z

I would rather not remove the parameter from that method signature. But instead add the usage of it like this;
change;
https://github.com/elastic/kibana/blob/master/test/support/page_objects/visualize_page.js#L532

from;
.findByCssSelector('circle[fill="#6eadc1"]')

to;
.findAllByCssSelector('circle[' + cssPart + ']')

And change any existing calls to getLineChartData to pass in 'fill="#6eadc1"'

The reason is that I have this modification in the private repo for integration testing where I make calls to get the line chart data for specific items using other attributes of the circles like this;

$ grep -r getLineChartData test/functional/*
test/functional/apps/metricbeat/_metricbeat.js:      return PageObjects.visualize.getLineChartData('data-label="CPU user space"');
test/functional/apps/packetbeat/_packetbeat.js:      return PageObjects.visualize.getLineChartData('data-label="Count"');
test/functional/apps/visualize/_line_chart.js:        return PageObjects.visualize.getLineChartData('fill="#57c17b"')

Right now this code isn't shared between the projects but it should be. I know it's not something we need in the existing Kibana tests as they stand right now. But the current line chart test is very simple with only a single line and this change allows us to test more complex charts. In fact other chart types should probably get a change like this too.

do not hardcode css-snippet

thomasneirynck · 2016-10-05T22:36:51Z

thanks @LeeDr. I reintroduced that change.

--------- **Commit 1:** remove unused vars do not hardcode css-snippet * Original sha: 56c130a * Authored by Thomas Neirynck <thomas@elastic.co> on 2016-10-05T15:47:05Z

LeeDr · 2016-10-05T22:53:47Z

@thomasneirynck did you run the Selenium tests locally? I think you may have copy/pasted something wrong. There shouldn't be backticks here;

-            .findByCssSelector('circle[fill="#6eadc1"]')
+            .findByCssSelector(`circle[${cssPart}]`)

thomasneirynck · 2016-10-05T22:58:22Z

@LeeDr I used the ES6 template literal syntax for this (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals). It's shorthand to avoid the string concatenation.

LeeDr · 2016-10-05T23:04:52Z

@thomasneirynck Cool. I learned something new! I thought it was a copy of the markdown syntax I used to indicate that as code! But now I see how it works.

[backport] PR #8558 to 5.x - Remove unused vars

[backport] PR #8558 to 5.0 - Remove unused vars

--------- **Commit 1:** remove unused vars do not hardcode css-snippet * Original sha: ff9027886aba1cc72821f5adfe4ced1a9c868c59 [formerly 56c130a] * Authored by Thomas Neirynck <thomas@elastic.co> on 2016-10-05T15:47:05Z Former-commit-id: 420dd2c

[backport] PR elastic#8558 to 5.x - Remove unused vars Former-commit-id: ec2f4a4

`101.3.0` ⏩ `101.4.0` [Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams) ## Changes This PR only updates tests and snapshots related to changes on EUI side: - updated `aria-current="true"` to `aria-current="page"` - updated icon usage `userAvatar` to `user` ## Package updates ### `@elastic/eui` #### [`v101.4.0`](https://github.com/elastic/eui/releases/v101.4.0) - Spread `labelProps` to the `label` element in `EuiCheckableCard` ([#8586](elastic/eui#8586)) - Add `controls`, `flask`, `comment`, and `readOnly` glyphs to `EuiIcon` ([#8580](elastic/eui#8580)) - Refactored `EuiExpression`, `EuiFacetGroup`, `EuiFacetButton`, `EuiFilterGroup`, `EuiHeader`, `EuiImage` and `EuiListGroup` to memoize their internal Emotion styles ([#8565](elastic/eui#8565)) - Updated global `border.radius.medium` token value for default `Borealis` theme to `4px` ([#8563](elastic/eui#8563)) - Updated `EuiProvider` to build themes including `highContrastMode` ([#8558](elastic/eui#8558)) **Accessibility** - Removed the `aria-label` attribute from the `ul` element in `EuiPagination` to avoid duplicate screen reader output ([#8597](elastic/eui#8597)) - Set a more specific `aria-current="page"` on list items in `EuiPagination` ([#8597](elastic/eui#8597)) - Added `aria-modal` to `EuiFlyout` with `type="overlay"` ([#8591](elastic/eui#8591)) **Dependency updates** - Updated `@elastic/prismjs-esql` to v1.1.0 ([#8587](elastic/eui#8587)) ### `@elastic/eui-theme-borealis@0.2.0` - Updated component tokens to use `computed` values to ensure correct inheritance from theme overrides ([#8558](elastic/eui#8558)) - Added `overrides.HCM` to `euiThemeBorealis` to support theme internal overrides ([#8558](elastic/eui#8558)) - Updated `border.radius.medium` token value to `4px` ([#8563](elastic/eui#8563)) ### `@elastic/eui-theme-common@0.2.0` - Added support for theme `overrides` as optional part of `EuiThemeShape` ([#8558](elastic/eui#8558)) - Updated `getComputed` to support high contrast mode overrides defined on `overrides.HCM` ([#8558](elastic/eui#8558)) --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

`101.3.0` ⏩ `101.4.0` [Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams) ## Changes This PR only updates tests and snapshots related to changes on EUI side: - updated `aria-current="true"` to `aria-current="page"` - updated icon usage `userAvatar` to `user` ## Package updates ### `@elastic/eui` #### [`v101.4.0`](https://github.com/elastic/eui/releases/v101.4.0) - Spread `labelProps` to the `label` element in `EuiCheckableCard` ([elastic#8586](elastic/eui#8586)) - Add `controls`, `flask`, `comment`, and `readOnly` glyphs to `EuiIcon` ([elastic#8580](elastic/eui#8580)) - Refactored `EuiExpression`, `EuiFacetGroup`, `EuiFacetButton`, `EuiFilterGroup`, `EuiHeader`, `EuiImage` and `EuiListGroup` to memoize their internal Emotion styles ([elastic#8565](elastic/eui#8565)) - Updated global `border.radius.medium` token value for default `Borealis` theme to `4px` ([elastic#8563](elastic/eui#8563)) - Updated `EuiProvider` to build themes including `highContrastMode` ([elastic#8558](elastic/eui#8558)) **Accessibility** - Removed the `aria-label` attribute from the `ul` element in `EuiPagination` to avoid duplicate screen reader output ([elastic#8597](elastic/eui#8597)) - Set a more specific `aria-current="page"` on list items in `EuiPagination` ([elastic#8597](elastic/eui#8597)) - Added `aria-modal` to `EuiFlyout` with `type="overlay"` ([elastic#8591](elastic/eui#8591)) **Dependency updates** - Updated `@elastic/prismjs-esql` to v1.1.0 ([elastic#8587](elastic/eui#8587)) ### `@elastic/eui-theme-borealis@0.2.0` - Updated component tokens to use `computed` values to ensure correct inheritance from theme overrides ([elastic#8558](elastic/eui#8558)) - Added `overrides.HCM` to `euiThemeBorealis` to support theme internal overrides ([elastic#8558](elastic/eui#8558)) - Updated `border.radius.medium` token value to `4px` ([elastic#8563](elastic/eui#8563)) ### `@elastic/eui-theme-common@0.2.0` - Added support for theme `overrides` as optional part of `EuiThemeShape` ([elastic#8558](elastic/eui#8558)) - Updated `getComputed` to support high contrast mode overrides defined on `overrides.HCM` ([elastic#8558](elastic/eui#8558)) --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit d2d2765) # Conflicts: # x-pack/solutions/security/plugins/security_solution/public/explore/components/paginated_table/__snapshots__/index.test.tsx.snap # x-pack/solutions/security/plugins/security_solution/public/explore/components/paginated_table/index.test.tsx # x-pack/solutions/security/plugins/security_solution/public/explore/hosts/components/uncommon_process_table/__snapshots__/index.test.tsx.snap # x-pack/solutions/security/plugins/security_solution/public/explore/network/components/network_dns_table/__snapshots__/index.test.tsx.snap # x-pack/solutions/security/plugins/security_solution/public/explore/network/components/network_http_table/__snapshots__/index.test.tsx.snap # x-pack/solutions/security/plugins/security_solution/public/explore/network/components/network_top_countries_table/__snapshots__/index.test.tsx.snap # x-pack/solutions/security/plugins/security_solution/public/explore/network/components/network_top_n_flow_table/__snapshots__/index.test.tsx.snap # x-pack/solutions/security/plugins/security_solution/public/explore/network/components/tls_table/__snapshots__/index.test.tsx.snap # x-pack/solutions/security/plugins/security_solution/public/explore/network/components/users_table/__snapshots__/index.test.tsx.snap

`101.3.0` ⏩ `101.4.0` [Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams) >[!IMPORTANT] This PR is a direct sibling to this [upgrade PR](#218778) to `main`. The difference is that it adds a standalone EUI package with the previous "Amsterdam" theme. Apart from the theme difference, **there are no further changes added**. ## Changes This PR only updates tests and snapshots related to changes on EUI side: - updated `aria-current="true"` to `aria-current="page"` - updated icon usage `userAvatar` to `user` ## Package updates ### `@elastic/eui` #### [`v101.4.0`](https://github.com/elastic/eui/releases/v101.4.0) - Spread `labelProps` to the `label` element in `EuiCheckableCard` ([#8586](elastic/eui#8586)) - Add `controls`, `flask`, `comment`, and `readOnly` glyphs to `EuiIcon` ([#8580](elastic/eui#8580)) - Refactored `EuiExpression`, `EuiFacetGroup`, `EuiFacetButton`, `EuiFilterGroup`, `EuiHeader`, `EuiImage` and `EuiListGroup` to memoize their internal Emotion styles ([#8565](elastic/eui#8565)) - Updated global `border.radius.medium` token value for default `Borealis` theme to `4px` ([#8563](elastic/eui#8563)) - Updated `EuiProvider` to build themes including `highContrastMode` ([#8558](elastic/eui#8558)) **Accessibility** - Removed the `aria-label` attribute from the `ul` element in `EuiPagination` to avoid duplicate screen reader output ([#8597](elastic/eui#8597)) - Set a more specific `aria-current="page"` on list items in `EuiPagination` ([#8597](elastic/eui#8597)) - Added `aria-modal` to `EuiFlyout` with `type="overlay"` ([#8591](elastic/eui#8591)) **Dependency updates** - Updated `@elastic/prismjs-esql` to v1.1.0 ([#8587](elastic/eui#8587)) ### `@elastic/eui-theme-borealis@0.2.0` - Updated component tokens to use `computed` values to ensure correct inheritance from theme overrides ([#8558](elastic/eui#8558)) - Added `overrides.HCM` to `euiThemeBorealis` to support theme internal overrides ([#8558](elastic/eui#8558)) - Updated `border.radius.medium` token value to `4px` ([#8563](elastic/eui#8563)) ### `@elastic/eui-theme-common@0.2.0` - Added support for theme `overrides` as optional part of `EuiThemeShape` ([#8558](elastic/eui#8558)) - Updated `getComputed` to support high contrast mode overrides defined on `overrides.HCM` ([#8558](elastic/eui#8558))

# Backport This will backport the following commits from `main` to `9.0`: - [Upgrade EUI to v101.4.0 (#218778)](#218778)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)

`101.3.0` ⏩ `101.4.0` [Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams) ## Changes This PR only updates tests and snapshots related to changes on EUI side: - updated `aria-current="true"` to `aria-current="page"` - updated icon usage `userAvatar` to `user` ## Package updates ### `@elastic/eui` #### [`v101.4.0`](https://github.com/elastic/eui/releases/v101.4.0) - Spread `labelProps` to the `label` element in `EuiCheckableCard` ([elastic#8586](elastic/eui#8586)) - Add `controls`, `flask`, `comment`, and `readOnly` glyphs to `EuiIcon` ([elastic#8580](elastic/eui#8580)) - Refactored `EuiExpression`, `EuiFacetGroup`, `EuiFacetButton`, `EuiFilterGroup`, `EuiHeader`, `EuiImage` and `EuiListGroup` to memoize their internal Emotion styles ([elastic#8565](elastic/eui#8565)) - Updated global `border.radius.medium` token value for default `Borealis` theme to `4px` ([elastic#8563](elastic/eui#8563)) - Updated `EuiProvider` to build themes including `highContrastMode` ([elastic#8558](elastic/eui#8558)) **Accessibility** - Removed the `aria-label` attribute from the `ul` element in `EuiPagination` to avoid duplicate screen reader output ([elastic#8597](elastic/eui#8597)) - Set a more specific `aria-current="page"` on list items in `EuiPagination` ([elastic#8597](elastic/eui#8597)) - Added `aria-modal` to `EuiFlyout` with `type="overlay"` ([elastic#8591](elastic/eui#8591)) **Dependency updates** - Updated `@elastic/prismjs-esql` to v1.1.0 ([elastic#8587](elastic/eui#8587)) ### `@elastic/eui-theme-borealis@0.2.0` - Updated component tokens to use `computed` values to ensure correct inheritance from theme overrides ([elastic#8558](elastic/eui#8558)) - Added `overrides.HCM` to `euiThemeBorealis` to support theme internal overrides ([elastic#8558](elastic/eui#8558)) - Updated `border.radius.medium` token value to `4px` ([elastic#8563](elastic/eui#8563)) ### `@elastic/eui-theme-common@0.2.0` - Added support for theme `overrides` as optional part of `EuiThemeShape` ([elastic#8558](elastic/eui#8558)) - Updated `getComputed` to support high contrast mode overrides defined on `overrides.HCM` ([elastic#8558](elastic/eui#8558)) --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

thomasneirynck force-pushed the chore/cleanup-test branch from 7c03f6c to 5c52173 Compare October 5, 2016 15:48

kobelb self-assigned this Oct 5, 2016

kobelb reviewed Oct 5, 2016

View reviewed changes

thomasneirynck force-pushed the chore/cleanup-test branch from 5c52173 to 2c35bfc Compare October 5, 2016 22:02

tylersmalley approved these changes Oct 5, 2016

View reviewed changes

remove unused vars

56c130a

do not hardcode css-snippet

thomasneirynck force-pushed the chore/cleanup-test branch from 2c35bfc to 56c130a Compare October 5, 2016 22:36

thomasneirynck merged commit 47565e7 into elastic:master Oct 5, 2016

This was referenced Oct 5, 2016

[backport] PR #8558 to 5.0 - Remove unused vars #8562

Merged

[backport] PR #8558 to 5.x - Remove unused vars #8563

Merged

elastic-jasper added a commit that referenced this pull request Oct 5, 2016

Backport PR #8558

420dd2c

--------- **Commit 1:** remove unused vars do not hardcode css-snippet * Original sha: 56c130a * Authored by Thomas Neirynck <thomas@elastic.co> on 2016-10-05T15:47:05Z

elastic-jasper added a commit that referenced this pull request Oct 5, 2016

Backport PR #8558

a14b51e

--------- **Commit 1:** remove unused vars do not hardcode css-snippet * Original sha: 56c130a * Authored by Thomas Neirynck <thomas@elastic.co> on 2016-10-05T15:47:05Z

thomasneirynck added a commit that referenced this pull request Oct 6, 2016

Merge pull request #8563 from elastic/jasper/backport/8558/5.x

ec2f4a4

[backport] PR #8558 to 5.x - Remove unused vars

thomasneirynck added a commit that referenced this pull request Oct 6, 2016

Merge pull request #8562 from elastic/jasper/backport/8558/5.0

8698aaf

[backport] PR #8558 to 5.0 - Remove unused vars

epixa added v5.1.0 v5.1.1 and removed v5.1.0 labels Dec 7, 2016

airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017

Merge pull request elastic#8563 from elastic/jasper/backport/8558/5.x

ed012f2

[backport] PR elastic#8558 to 5.x - Remove unused vars Former-commit-id: ec2f4a4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove unused vars#8558

Remove unused vars#8558
thomasneirynck merged 1 commit intoelastic:masterfrom
thomasneirynck:chore/cleanup-test

thomasneirynck commented Oct 5, 2016

Uh oh!

kobelb Oct 5, 2016

Uh oh!

thomasneirynck commented Oct 5, 2016 •

edited

Loading

Uh oh!

kobelb commented Oct 5, 2016

Uh oh!

LeeDr commented Oct 5, 2016

Uh oh!

thomasneirynck commented Oct 5, 2016

Uh oh!

LeeDr commented Oct 5, 2016 •

edited

Loading

Uh oh!

thomasneirynck commented Oct 5, 2016

Uh oh!

LeeDr commented Oct 5, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

thomasneirynck commented Oct 5, 2016

Uh oh!

kobelb Oct 5, 2016

Choose a reason for hiding this comment

Uh oh!

thomasneirynck commented Oct 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kobelb commented Oct 5, 2016

Uh oh!

LeeDr commented Oct 5, 2016

Uh oh!

thomasneirynck commented Oct 5, 2016

Uh oh!

LeeDr commented Oct 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thomasneirynck commented Oct 5, 2016

Uh oh!

LeeDr commented Oct 5, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

thomasneirynck commented Oct 5, 2016 •

edited

Loading

LeeDr commented Oct 5, 2016 •

edited

Loading