-
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
Legacy url conflict UI improvements #114172
Legacy url conflict UI improvements #114172
Conversation
Now instead of taking a raw JSON string, the component requires individual parameters and it generates the JSON string on its own. It also makes use of the SpacesManager to find the active spaces so that consumers don't have to do so.
Includes changes to text and additional docs link.
Standardized the naming after all of the refactoring and other changes.
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.
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.
Tested locally, works great! I like the copy improvements quite a bit, nice job 🏅.
I have a few comments below, but nothing blocking.
x-pack/plugins/spaces/public/legacy_urls/components/embeddable_legacy_url_conflict_internal.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/spaces/public/legacy_urls/components/embeddable_legacy_url_conflict_internal.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/spaces/public/legacy_urls/components/legacy_url_conflict_internal.tsx
Outdated
Show resolved
Hide resolved
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.
dockLink changes LGTM.
FYI, I love the new UI copy, it's got everything one would need to sort out the url conflict issue!
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 - nice updates to the copy to make the error more understandable
code review
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Public APIs missing exports
Page load bundle
History
To update your PR or re-run it, just comment with: |
💔 Backport failed
To backport manually run: |
# Conflicts: # docs/development/core/public/kibana-plugin-core-public.doclinksstart.md
Resolves #113470 and #113407.
Testing
This PR changes how the EmbeddableLegacyUrlConflict component functions.
I created the following minimal staging data to exercise embeddable maps and lens visualizations:
Testing steps:
Screenshots:
Regular legacy URL conflict (mouse over the button to reveal the tooltip):
Embeddable legacy URL conflict (left is map, right is lens):
Embeddable legacy URL conflict (after clicking "View details"):
Note to self: update dev guide with new screenshots before merging.