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

[App Search] Convert DocumentCreationModal to DocumentCreationFlyout #86508

Merged
merged 3 commits into from
Dec 18, 2020
Merged

[App Search] Convert DocumentCreationModal to DocumentCreationFlyout #86508

merged 3 commits into from
Dec 18, 2020

Conversation

cee-chen
Copy link
Member

Summary

I was brainstorming with @daveyholler yesterday on how best to show form feedback errors on DocumentCreationModal. Here's the current problem with it:

As you can see, on small/laptop screens, errors are hidden in the overflowing modal body and are super difficult to see. This is pretty awful UX and additionally not what modals are suited for per EUI's guidelines:

Avoid scrolling content
Modal content should fit in a single view. If your modal has a lot of detail or a long list of items, consider a different solution, such as a form or a table.

The advantages we get from switching to EuiFlyout instead are numerous:

  • We get (almost) the full height / vertical real estate of the screen (sans Kibana's top headers)
  • We get EuiFlyout's banner prop, which makes displaying form errors SUPER easy and awesome, and is never hidden by scrolling
  • EuiFlyout's can be dismissed by clicking outside the flyout by default unlike EuiModal (I personally love this feature so much I can't even say, I hate modals that can't be dismissed by clicking outside them :D)

Screencaps

f13se63bA4

Checklist

@cee-chen
Convert DocumentCreationModal to a Flyout
24e795c 
- Per discussion w/ Davey - it handles longer/detailed content better
@cee-chen
Update instances referencing DocumentCreationFlyout
f487f11
@cee-chen
Update flyout children
de6015e 
- modal->flyout
- add hasBorder, set EuiTitle sizes, add flexgroup to footer buttons
@cee-chen cee-chen added Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.12.0 labels Dec 18, 2020
@cee-chen cee-chen requested review from daveyholler, JasonStoltz and a team December 18, 2020 19:00
daveyholler
daveyholler approved these changes Dec 18, 2020
View reviewed changes
Copy link
Contributor

@daveyholler daveyholler left a comment

Choose a reason for hiding this comment

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

Such awesome! I'm excited for this!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 1.7MB 1.7MB +1.0KB

Distributable file count

id before after diff
default 47298 48058 +760

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

@cee-chen cee-chen merged commit 122f1e1 into elastic:master Dec 18, 2020
@cee-chen cee-chen deleted the document-creation-flyout branch December 18, 2020 21:31
cee-chen pushed a commit to cee-chen/kibana that referenced this pull request Dec 18, 2020
@cee-chen
[App Search] Convert DocumentCreationModal to DocumentCreationFlyout (e…
bbb18a8 
…lastic#86508)

* Convert DocumentCreationModal to a Flyout

- Per discussion w/ Davey - it handles longer/detailed content better

* Update instances referencing DocumentCreationFlyout

* Update flyout children

- modal->flyout
- add hasBorder, set EuiTitle sizes, add flexgroup to footer buttons
@cee-chen cee-chen mentioned this pull request Dec 18, 2020
Merged
cee-chen pushed a commit that referenced this pull request Dec 18, 2020
[App Search] Convert DocumentCreationModal to DocumentCreationFlyout (#…
9eb1ad5 
…86508) (#86542)

* Convert DocumentCreationModal to a Flyout

- Per discussion w/ Davey - it handles longer/detailed content better

* Update instances referencing DocumentCreationFlyout

* Update flyout children

- modal->flyout
- add hasBorder, set EuiTitle sizes, add flexgroup to footer buttons
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 21, 2020
@gmmorris
Merge branch 'master' into alerts/remove-generics-defaults-redux
561ebb1 
* master: (48 commits)
  Fix request with disabled aggregation (elastic#85696)
  [Security Solution][Detections][Threshold Rules] Threshold Rule Bug Fixes (elastic#84918)
  Removed a possibility to define two different names for Alert types on API and UI level. (elastic#86236)
  Bump Node.js from version 14.15.2 to 14.15.3 (elastic#86593)
  [index patterns] Fleep app - Keep saved object field list until field caps provides fields (elastic#85370)
  [Security Solutions] fix timeline tabs + layout (elastic#86581)
  Upgrade to hapi version 20 (elastic#85406)
  App Services: Remove remaining uiActions, expressions, data, embeddable circular dependencies. (elastic#82791)
  Rename chartLibrary setting to legacyChartsLibrary (elastic#86529)
  [CI] TeamCity updates (elastic#85843)
  [Maps] Use Json for mvt-tests (elastic#86492)
  [Rollup Jobs] Added autofocus to cron editor (elastic#86324)
  [Monitoring][Alerting] CCR read exceptions alert (elastic#85908)
  [CI] Bump memory for main CI workers (elastic#86541)
  Explicitly set Elasticsearch heap size during CI and local development (elastic#86513)
  [App Search] Updates to results on the documents view (elastic#86181)
  [Discover] Change default sort handling  (elastic#85561)
  [App Search] Convert DocumentCreationModal to DocumentCreationFlyout (elastic#86508)
  [App Search] Sample Engines should have access to the Crawler (elastic#86502)
  Fixed duplication of create new modal (elastic#86489)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JasonStoltz JasonStoltz JasonStoltz approved these changes

@daveyholler daveyholler daveyholler approved these changes

Assignees
No one assigned
Labels
Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.12.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cee-chen @kibanamachine @daveyholler @JasonStoltz