[Discover] Deangularize timechart header by stratoula · Pull Request #66532 · elastic/kibana

stratoula · 2020-05-14T09:58:02Z

Summary

Fixes #65432

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility (https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)

Screenshots

Before:

Now:

src/plugins/discover/public/application/components/timechart_header/timechart_header.tsx

stratoula · 2020-05-14T16:06:03Z

@elasticmachine merge upstream

stratoula · 2020-05-15T06:34:11Z

@elasticmachine merge upstream

elasticmachine · 2020-05-15T08:46:20Z

Pinging @elastic/kibana-app (Team:KibanaApp)

src/plugins/discover/public/application/components/timechart_header/timechart_header.tsx

Mainly moved interval notice to an `append` and copy updates

stratoula · 2020-05-18T06:42:57Z

@elasticmachine merge upstream

src/plugins/discover/public/application/angular/discover.js

kertal

Code LGTM 👍 , tested locally in Chrome, Firefox, Safari, works as expected, no regression detected. Added a cleanup suggestion. Thx for paying attention to details like the missing warning icon. Those angular directives vanished without any warning.

stratoula · 2020-05-18T12:43:45Z

@kertal thanks for the suggestion. Will wait for the design team approval and merge it!!

cchaos

LGTM! Sorry about breaking those tests. The append works well now, I think. Thanks!

kertal · 2020-05-18T12:48:00Z

@cchaos I've one question about wording, since I'm not a native speaker

Hourly -> Hour, Daily -> Day, ...

right?

Maybe it would make sense to add the "per" prefix into the select box?

Auto
Per millisecond
Per second
....

In English & German, it know it's similar, per {variable}, but there could be languages where it's e.g. {variable} per

cchaos · 2020-05-18T12:56:16Z

Ah yes, good catch. I don't think we should move the "per" to be part of the selection. I think we should try to make sure the selections themselves are all similarly phrased.

Looping @KOTungseth in here.

Should we change all of the selections to not have the "ly" affix? So it reads like:

per [millisecond]
per [day]
per [hour]
...etc

KOTungseth · 2020-05-19T13:22:28Z

I agree with @cchaos that we should remove ly from the time filter options. From what I've been taught, this list (traditionally) should appear as:
Auto
Millisecond
Second
Minute
Hour
Day
Week
Month
Year

stratoula · 2020-05-19T13:23:15Z

Thank you @KOTungseth, will proceed with the change!

kibanamachine · 2020-05-19T17:56:16Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: b535961

History

💔 Build #48956 failed 3a0ef5889988ef068f3f8e2f1853c192b4e56ecc
💔 Build #48937 failed 0193218e0e57bf488d863f85ef0b0d14353385b6
💚 Build #48632 succeeded 292e8f5
💚 Build #48601 succeeded e08e3dc
💔 Build #48593 failed f7de36c

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

streamich

AppArch code changes LGTM.

* Deangularize timechart header * Add label attr to options * Watch the interval prop change * Tweaking the UI Mainly moved interval notice to an `append` and copy updates * Remove outdated i18n tokens * fix functional test * Change functional test due to dom changes * fix functional test * remove unecessary translation * remove watcher as it is not necessary anymore * change interval options copies Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: cchaos <caroline.horn@elastic.co>

* Deangularize timechart header * Add label attr to options * Watch the interval prop change * Tweaking the UI Mainly moved interval notice to an `append` and copy updates * Remove outdated i18n tokens * fix functional test * Change functional test due to dom changes * fix functional test * remove unecessary translation * remove watcher as it is not necessary anymore * change interval options copies Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: cchaos <caroline.horn@elastic.co> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: cchaos <caroline.horn@elastic.co>

* master: (33 commits) [Saved Objects] adds support for including hidden types in saved objects client (elastic#66879) [Discover] Deangularize timechart header (elastic#66532) [Discover] Improve and unskip a11y context view test (elastic#66959) [SIEM] Refactor Timeline.timelineType draft to Timeline.status draft (elastic#66864) docs: update RUM documentation link (elastic#67042) [QA] fixup coverage ingestion tests. (elastic#66905) [Metrics UI] Add support for multiple groupings to Metrics Explorer (and Alerts) (elastic#66503) [Metrics UI] Add sorting for name and value to Inventory View (elastic#66644) [Metrics UI] Change Metric Threshold Alert charts to use bar charts (elastic#66672) [Uptime] Use React.lazy for alert type registration (elastic#66829) [Reporting] Consolidate API Integration Test configs (elastic#66637) Allow histogram fields in average and sum aggregations (elastic#66891) Fix saved object share link (elastic#66771) move role reset into the top level after clause (elastic#66971) Automate the labels for any PRs affecting files for the Ingest Management team (elastic#67022) [SIEMDPOINT] Move endpoint to siem (elastic#66907) server.uuid so is not used (elastic#66963) Revert "[ci/stats] fix git metadata collection (elastic#66840)" [Uptime] Unmount uptime app properly (elastic#66950) [Visualize] Bar chart: Show missing values on chart setting (elastic#66375) ...

…ent/add-support-in-url-for-hidden-toggle * 'master' of github.com:elastic/kibana: (49 commits) [Uptime] Improve responsiveness details page (elastic#67034) skip flaky suite (elastic#66669) Revert "Integration of a static filesystem for the node_modules (elastic#47998)" (elastic#67124) Support api_integration/kibana/stats against remote hosts (elastic#53000) chore(NA): add module name mapper for src plugins on x-pack (elastic#67103) Change the error message on TSVB in order to be more user friendly (elastic#67090) [kbn/optimizer] poll parent process to avoid zombie processes (elastic#67059) [Visualize] Lazy load default editor, fix duplicated styles (elastic#66732) Bump styled-component dependencies (elastic#66611) Bump react-markdown dependencies (elastic#66615) Fix Core docs links (elastic#66977) Timelion graph is not refreshing content after searching or filtering (elastic#67023) Remove `--xpack.endpoint.enabled=true` from README.md file (elastic#67053) Move apm tutorial from apm plugin into apm_oss plugin (elastic#66432) [Logs UI] Restore call to `UsageCollector.countLogs` (elastic#67051) Remove unused license check result from LP Security plugin (elastic#66966) [Saved Objects] adds support for including hidden types in saved objects client (elastic#66879) [Discover] Deangularize timechart header (elastic#66532) [Discover] Improve and unskip a11y context view test (elastic#66959) [SIEM] Refactor Timeline.timelineType draft to Timeline.status draft (elastic#66864) ... # Conflicts: # x-pack/plugins/index_management/__jest__/client_integration/helpers/home.helpers.ts

stratoula self-assigned this May 14, 2020

stratoula added Feature:Discover Discover Application v8.0.0 labels May 14, 2020

stratoula commented May 14, 2020

View reviewed changes

src/plugins/discover/public/application/components/timechart_header/timechart_header.tsx Outdated Show resolved Hide resolved

stratoula force-pushed the discover/deangularize-datetime-histogram branch from afdd1c8 to 7f03a83 Compare May 14, 2020 10:07

Deangularize timechart header

2785e5a

stratoula force-pushed the discover/deangularize-datetime-histogram branch from a0fc6cd to 2785e5a Compare May 14, 2020 13:24

Add label attr to options

88ae7bd

Merge branch 'master' into discover/deangularize-datetime-histogram

ba4ca56

Merge branch 'master' into discover/deangularize-datetime-histogram

15fe2cf

stratoula marked this pull request as ready for review May 15, 2020 08:45

stratoula requested a review from a team May 15, 2020 08:45

stratoula added release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// labels May 15, 2020

stratoula requested review from a team and cchaos and removed request for cchaos May 15, 2020 09:58

stratoula added the v7.9.0 label May 15, 2020

kertal reviewed May 15, 2020

View reviewed changes

src/plugins/discover/public/application/components/timechart_header/timechart_header.tsx Show resolved Hide resolved

src/plugins/discover/public/application/components/timechart_header/timechart_header.tsx Show resolved Hide resolved

stratoula and others added 4 commits May 15, 2020 18:27

Watch the interval prop change

613d851

Tweaking the UI

5a891c4

Mainly moved interval notice to an `append` and copy updates

Remove outdated i18n tokens

96b4543

fix functional test

657b78b

elasticmachine and others added 2 commits May 17, 2020 23:43

Merge branch 'master' into discover/deangularize-datetime-histogram

0cb3955

Change functional test due to dom changes

9782531

stratoula force-pushed the discover/deangularize-datetime-histogram branch from ffeaaa5 to 9782531 Compare May 18, 2020 08:27

stratoula added 2 commits May 18, 2020 12:13

fix functional test

f7de36c

remove unecessary translation

e08e3dc

stratoula requested a review from kertal May 18, 2020 12:28

kertal reviewed May 18, 2020

View reviewed changes

src/plugins/discover/public/application/angular/discover.js Show resolved Hide resolved

kertal approved these changes May 18, 2020

View reviewed changes

remove watcher as it is not necessary anymore

292e8f5

cchaos approved these changes May 18, 2020

View reviewed changes

stratoula requested a review from a team as a code owner May 19, 2020 13:38

change interval options copies

b535961

stratoula force-pushed the discover/deangularize-datetime-histogram branch from 3a0ef58 to b535961 Compare May 19, 2020 15:54

streamich approved these changes May 20, 2020

View reviewed changes

stratoula merged commit 42d21bc into elastic:master May 20, 2020

stratoula deleted the discover/deangularize-datetime-histogram branch May 20, 2020 07:29

stratoula mentioned this pull request May 20, 2020

[7.x] [Discover] Deangularize timechart header (#66532) #67078

Merged

Conversation

stratoula commented May 14, 2020 • edited by cchaos Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Screenshots

Uh oh!

Uh oh!

stratoula commented May 14, 2020

Uh oh!

stratoula commented May 15, 2020

Uh oh!

elasticmachine commented May 15, 2020

Uh oh!

Uh oh!

Uh oh!

stratoula commented May 18, 2020

Uh oh!

Uh oh!

kertal left a comment

Choose a reason for hiding this comment

Uh oh!

stratoula commented May 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

kertal commented May 18, 2020

Uh oh!

cchaos commented May 18, 2020

Uh oh!

KOTungseth commented May 19, 2020

Uh oh!

stratoula commented May 19, 2020

Uh oh!

kibanamachine commented May 19, 2020

💚 Build Succeeded

History

Uh oh!

streamich left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

stratoula commented May 14, 2020 •

edited by cchaos

Loading

stratoula commented May 18, 2020 •

edited

Loading