Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Visualizations] Update the texts on the wizard #82926

Merged
merged 10 commits into from
Nov 24, 2020
Merged

[Visualizations] Update the texts on the wizard #82926

merged 10 commits into from
Nov 24, 2020

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Nov 9, 2020 •
edited
Loading

Summary

Updates the texts on the wizard for the legacy visualizations.
Screenshot 2020-11-16 at 6 17 25 PM

Checklist

Delete any items that are not applicable to this PR.

@stratoula stratoula requested a review from gchaps November 9, 2020 08:56
@stratoula stratoula added Feature:Visualizations Generic visualization features (in case no more specific feature label is available) v7.11.0 v8.0.0 release_note:enhancement labels Nov 9, 2020
@stratoula stratoula mentioned this pull request Nov 10, 2020
Merged
6 tasks
@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula stratoula requested a review from sulemanof November 16, 2020 16:22
@apmmachine
Copy link
Contributor

apmmachine commented Nov 16, 2020 •
edited
Loading

💚 Build Succeeded

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

gchaps
gchaps reviewed Nov 16, 2020
View reviewed changes
Copy link
Contributor

@gchaps gchaps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heat map should be:

Shade data in cells in a matrix.

@stratoula
Copy link
Contributor Author

Shade data in cells in a matrix.

Ooops, good catch 😄

@stratoula stratoula marked this pull request as ready for review November 17, 2020 09:59
@stratoula stratoula requested a review from a team November 17, 2020 09:59
@stratoula stratoula added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Nov 17, 2020
@elasticmachine
Copy link
Contributor

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

sulemanof
sulemanof reviewed Nov 18, 2020
View reviewed changes
Copy link
Contributor

@sulemanof sulemanof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Text changes looks good to me 👍
Just a question: could you please mention in the PR description the reason of text changes?

If I remember properly, we didn't change the visualization types names to Sentence case since they are feature names - hence should have the Title case as EUI Writing guidelines
says. But I might be wrong, of course 😕

@stratoula
Copy link
Contributor Author

We changed them as the texts were not so explanatory to help the users understand what they can do with each chart. Now about the titles, Gail suggested this change and I prefer it that way. But I don't know the history behind it tbh. @cchaos can you give us feedback on that?

@cchaos
Copy link
Contributor

cchaos commented Nov 18, 2020

I wouldn't consider these chart types to specifically be "features" and certainly they're not top-level branded features. Sentence case is the right approach here.

@stratoula
Copy link
Contributor Author

Thank you Caroline 🙂

@gchaps
Copy link
Contributor

gchaps commented Nov 18, 2020 •
edited
Loading

Another reason for the text change is so that all the descriptions are presented in the same way, starting with a verb.

@gchaps
Copy link
Contributor

gchaps commented Nov 18, 2020

@stratoula @cchaos One other thought. What is the action that brings the user to this view. Should the title be consistent with that action? Meaning "Add visualization" or "Add panel"

@stratoula
Copy link
Contributor Author

The action is Create Visualization. You mean instead of having New Visualization to have Create Visualization? For me it makes sense to leave it as it is but I don't have a strong opinion on that.

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@gchaps
Copy link
Contributor

gchaps commented Nov 19, 2020

@stratoula Let's leave as is, then.

@cchaos
Copy link
Contributor

cchaos commented Nov 19, 2020

I think both flows (from dashboard as a panel and from Visualize as a visualization) use the same modal so I think it's hard to make the distinction here. I'm fine with visualization as it's what groups these items.

@flash1293
Copy link
Contributor

@elasticmachine merge upstream

flash1293
flash1293 approved these changes Nov 23, 2020
View reviewed changes
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
visTypeMetric 25.9KB 25.9KB -2.0B
visTypeTable 18.9KB 18.9KB +8.0B
visTypeTagcloud 19.6KB 19.5KB -15.0B
visTypeTimelion 34.8KB 34.8KB -13.0B
visTypeVislib 66.0KB 66.0KB +12.0B
total -10.0B

History

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

@stratoula stratoula merged commit cac95a3 into elastic:master Nov 24, 2020
@stratoula stratoula mentioned this pull request Nov 24, 2020
Merged
stratoula added a commit to stratoula/kibana that referenced this pull request Nov 24, 2020
@stratoula @kibanamachine
[Visualizations] Update the texts on the wizard (elastic#82926)
1a07091 
* [Visualizations] Update the texts on the wizard

* Fix functional test

* Final texts

* Fix heatmap description

Co-authored-by: Kibana Machine <[email protected]>
stratoula added a commit that referenced this pull request Nov 24, 2020
@stratoula @kibanamachine
[Visualizations] Update the texts on the wizard (#82926) (#84175)
2ea288f 
* [Visualizations] Update the texts on the wizard

* Fix functional test

* Final texts

* Fix heatmap description

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gchaps gchaps gchaps left review comments

@sulemanof sulemanof sulemanof approved these changes

@flash1293 flash1293 flash1293 approved these changes

Assignees
No one assigned
Labels
Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.11.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@stratoula @apmmachine @elasticmachine @cchaos @gchaps @flash1293 @kibanamachine @sulemanof