Add url overflow handling to KP #67899

joshdover · 2020-06-01T20:04:54Z

Summary

This moves the previously Angular-only URL overflow handling to Core's global router and adds a new "error" application that will be displayed when a user encounters a very long URL.

The UI has been mostly unchanged, but it could really benefit from:

A copy update
A design pass
Functional tests added

However, I'm keen on getting this in for the next (last?) 7.8 BC and will iterate on this as a follow up.

Screenshot

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
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-06-03T16:20:50Z

Pinging @elastic/kibana-platform (Team:Platform)

joshdover · 2020-06-03T16:32:47Z

src/core/public/core_app/errors/url_overflow_ui.tsx

+            values={{
+              storeInSessionStorageConfig: <code>state:storeInSessionStorage</code>,
+              kibanaSettingsLink: (
+                <a href={basePath.prepend('/app/management/kibana/settings')}>


This will need to be changed when backporting to 7.8 branch

joshdover · 2020-06-03T16:33:11Z

src/core/public/core_app/errors/url_overflow.tsx

+            values={{
+              storeInSessionStorageParam: <code>state:storeInSessionStorage</code>,
+              advancedSettingsLink: (
+                <a href={basePath.prepend('/app/management/kibana/settings')}>


This will need to be changed when backporting to 7.8

...lugins/maps/public/connected_components/map/features_tooltip/feature_geometry_filter_form.js

src/core/public/core_app/errors/url_overflow_ui.tsx

src/core/public/application/application_service.mock.ts

src/core/public/core_app/core_app.ts

src/core/public/core_app/errors/error_application.tsx

pgayvallet · 2020-06-03T18:50:10Z

src/core/public/core_app/errors/error_application.tsx

+  const [currentLocation, setCurrentLocation] = useState(history.location);
+  useLayoutEffect(() => history.listen((location) => setCurrentLocation(location)), [history]);
+
+  const searchParams = new URLSearchParams(currentLocation.search);


URLSearchParams is not compatible with IE11: https://caniuse.com/#search=URLSearchParams (unless we have a shim?) Maybe use the 'query-string' module instead?

There is a core-js polyfill, but I will double check it works in IE 👍

src/core/public/core_app/errors/error_application.tsx

gchaps · 2020-06-03T22:59:24Z

Here is my suggestion for copy:

The URL for this dashboard is too long,
and we can't display the dashboard.

Things to try:

Enable the state:storeInSession option in Advanced Settings.
Remove content from your dashboard. This error occurs only for complex dashboards.

nreese

maps changes LGTM
code review

joshdover · 2020-06-04T22:55:36Z

x-pack/plugins/translations/translations/ja-JP.json

@@ -138,16 +138,6 @@
    "charts.controls.rangeErrorMessage": "値は{min}と{max}の間でなければなりません",
    "charts.controls.vislibBasicOptions.legendPositionLabel": "凡例位置",
    "charts.controls.vislibBasicOptions.showTooltipLabel": "ツールヒントを表示",
-    "common.ui.errorUrlOverflow.breadcrumbs.errorText": "エラー",


I changed the defaultMessage of these strings significantly, so I figured I should delete the old translations so they get updated. Is that the right process @Bamieh ?

Implemented design changes

gchaps · 2020-06-04T23:06:45Z

Some fixes to the updated copy:

Remove "the" before Advanced Settings.
Use a bulleted list instead of a numbered list.
Remove "Whoa there". It doesn't add any value and is misspelled. Use this for the heading instead:

The URL for this dashboard is too long,
and we can't display the dashboard.

If the two-line title is too much, we can work on it.

src/core/public/core_app/errors/url_overflow_ui.tsx

flash1293

Kibana app code changes LGTM. Left one nit about the error page itself.

Not sure whether that's expected but when the error case is triggered, the error page is shown very briefly, then the page reloads.

joshdover · 2020-06-08T18:26:22Z

Kibana app code changes LGTM. Left one nit about the error page itself.

Not sure whether that's expected but when the error case is triggered, the error page is shown very briefly, then the page reloads.

Will switch to using window.location.assign to eliminate that flashing. Also working with @gchaps on making the copy not dashboard-specific.

kibanamachine · 2020-06-08T20:28:54Z

💚 Build Succeeded

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

History

💔 Build #51954 failed f989d80
💔 Build #51586 failed a46af84
💔 Build #51455 failed 9b6a31f
💔 Build #51240 failed 7c74b68b4daaa5f11bc043b9912d043ea9b8c1d5
💔 Build #50944 failed 21d0ac9f2dd2be0e8177fe36fe3bbd98ae85fba8

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

# Conflicts: # src/legacy/ui/public/_index.scss # src/plugins/kibana_legacy/public/angular/angular_config.tsx # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json

joshdover force-pushed the issue-67776 branch from d11f719 to 21d0ac9 Compare June 1, 2020 21:53

joshdover mentioned this pull request Jun 2, 2020

Discuss: Ui settings ownership #63459

Closed

joshdover force-pushed the issue-67776 branch from 21d0ac9 to 7c74b68 Compare June 2, 2020 22:05

Add url overflow handling to KP

9b6a31f

joshdover force-pushed the issue-67776 branch from 7c74b68 to 9b6a31f Compare June 3, 2020 16:17

joshdover added Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.8.0 v7.9.0 labels Jun 3, 2020

joshdover marked this pull request as ready for review June 3, 2020 16:20

joshdover requested a review from a team June 3, 2020 16:20

joshdover requested review from a team as code owners June 3, 2020 16:20

joshdover commented Jun 3, 2020

View reviewed changes

nreese reviewed Jun 3, 2020

View reviewed changes

...lugins/maps/public/connected_components/map/features_tooltip/feature_geometry_filter_form.js Outdated Show resolved Hide resolved

cchaos previously requested changes Jun 3, 2020

View reviewed changes

src/core/public/core_app/errors/url_overflow_ui.tsx Outdated Show resolved Hide resolved

src/core/public/core_app/errors/url_overflow_ui.tsx Outdated Show resolved Hide resolved

pgayvallet reviewed Jun 3, 2020

View reviewed changes

joshdover added 3 commits June 3, 2020 15:50

PR comments

6e391c9

Merge remote-tracking branch 'upstream/master' into issue-67776

f7c6fa9

Update i18n ids

a46af84

pgayvallet approved these changes Jun 4, 2020

View reviewed changes

nreese approved these changes Jun 4, 2020

View reviewed changes

joshdover added 2 commits June 4, 2020 16:21

Merge remote-tracking branch 'upstream/master' into issue-67776

3e20242

Update copy

f989d80

joshdover commented Jun 4, 2020

View reviewed changes

flash1293 reviewed Jun 5, 2020

View reviewed changes

src/core/public/core_app/errors/url_overflow_ui.tsx Outdated Show resolved Hide resolved

flash1293 reviewed Jun 5, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into issue-67776

93d9f93

joshdover added 2 commits June 8, 2020 12:43

Use location.assign + update copy

178136d

Fix tests

3046985

joshdover merged commit f090775 into elastic:master Jun 8, 2020

joshdover deleted the issue-67776 branch June 8, 2020 21:03

joshdover mentioned this pull request Jun 8, 2020

[7.x] Add url overflow handling to KP (#67899) #68573

Merged

joshdover added a commit to joshdover/kibana that referenced this pull request Jun 8, 2020

Add url overflow handling to KP (elastic#67899)

3d0e305

joshdover mentioned this pull request Jun 8, 2020

[7.8] Add url overflow handling to KP (#67899) #68578

Closed

joshdover added a commit that referenced this pull request Jun 8, 2020

[7.x] Add url overflow handling to KP (#67899) (#68573)

77f74ec

joshdover removed the v7.8.0 label Jun 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add url overflow handling to KP #67899

Add url overflow handling to KP #67899

joshdover commented Jun 1, 2020 •

edited

Loading

elasticmachine commented Jun 3, 2020

joshdover Jun 3, 2020

joshdover Jun 3, 2020

pgayvallet Jun 3, 2020

joshdover Jun 3, 2020

gchaps commented Jun 3, 2020

nreese left a comment

joshdover Jun 4, 2020

gchaps commented Jun 4, 2020

flash1293 left a comment

joshdover commented Jun 8, 2020

kibanamachine commented Jun 8, 2020

Add url overflow handling to KP #67899

Add url overflow handling to KP #67899

Conversation

joshdover commented Jun 1, 2020 • edited Loading

Summary

Screenshot

Checklist

For maintainers

elasticmachine commented Jun 3, 2020

joshdover Jun 3, 2020

Choose a reason for hiding this comment

joshdover Jun 3, 2020

Choose a reason for hiding this comment

pgayvallet Jun 3, 2020

Choose a reason for hiding this comment

joshdover Jun 3, 2020

Choose a reason for hiding this comment

gchaps commented Jun 3, 2020

nreese left a comment

Choose a reason for hiding this comment

joshdover Jun 4, 2020

Choose a reason for hiding this comment

gchaps commented Jun 4, 2020

flash1293 left a comment

Choose a reason for hiding this comment

joshdover commented Jun 8, 2020

kibanamachine commented Jun 8, 2020

💚 Build Succeeded

History

joshdover commented Jun 1, 2020 •

edited

Loading