-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Discover] Deangularize timechart header #66532
[Discover] Deangularize timechart header #66532
Conversation
src/plugins/discover/public/application/components/timechart_header/timechart_header.tsx
Outdated
Show resolved
Hide resolved
afdd1c8
to
7f03a83
Compare
a0fc6cd
to
2785e5a
Compare
@elasticmachine merge upstream |
@elasticmachine merge upstream |
Pinging @elastic/kibana-app (Team:KibanaApp) |
src/plugins/discover/public/application/components/timechart_header/timechart_header.tsx
Outdated
Show resolved
Hide resolved
src/plugins/discover/public/application/components/timechart_header/timechart_header.tsx
Outdated
Show resolved
Hide resolved
Mainly moved interval notice to an `append` and copy updates
@elasticmachine merge upstream |
ffeaaa5
to
9782531
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@kertal thanks for the suggestion. Will wait for the design team approval and merge it!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Sorry about breaking those tests. The append
works well now, I think. Thanks!
@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?
In English & German, it know it's similar, |
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:
|
I agree with @cchaos that we should remove |
Thank you @KOTungseth, will proceed with the change! |
3a0ef58
to
b535961
Compare
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Summary
Fixes #65432
Checklist
Delete any items that are not applicable to this PR.
Screenshots
Before:
Now: