-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Time to Visualize] Align Lens & Visualize Top nav Buttons & Behaviour #86922
[Time to Visualize] Align Lens & Visualize Top nav Buttons & Behaviour #86922
Conversation
@elasticmachine merge upstream |
Pinging @elastic/kibana-presentation (Team:Presentation) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@ThomThomson I have seen something that is weird but has to do with the breadcrumbs PR. Sorry about that, I should have reported it there but I saw it now. I have the flag set to true and I create a Lens chart. Everything is fine. When I set the flag to false and edit the Lens viz I see the Is it something expected? (and ofc is irrelevant with this PR, I am just asking and if you feel that is sthg that should be fixed then I will create an issue) |
((originatingApp && originatingApp === 'dashboards') || originatingApp === 'canvas') && | ||
(hasUnappliedChanges || hasUnsavedChanges) | ||
originatingApp && | ||
(hasUnappliedChanges || hasUnsavedChanges) && |
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.
Much cleaner!
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
What's happening here is that you are getting into an undefined state by editing a previously created by value visualization with the It is probably worth raising an issue for this, even though it's a fairly unlikely edge case. Thanks for the review! |
I agree @ThomThomson. I will raise an issue. Thanx for explaining this. |
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 it locally and it works fine 👏
elastic#86922) * Aligned Lens & Visualize Top nav behaviour and look
Summary
Fixes #81246 by aligning the wording and behaviour of the top nav buttons between lens and visualize.
Checklist
Delete any items that are not applicable to this PR.
For maintainers