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.

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

[Maps] Quick design fixes for 7.3 #39429

Merged
merged 9 commits into from
Jun 27, 2019
Merged

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Jun 21, 2019

This looks like a long list, but just because I was trying to be thorough documenting what I changed, but really it was mostly spacing and component misuse edits.

Also, please excuse a lot of the diff's because my linter/prettier auto-fixed a lot of the formatting.

Listing page changes

1. Fixed the title alignment

2. Altered the help dropdown link to GH

File upload layout changes

3. File chooser screen: Moved file guidelines to help text

4. File chooser screen: Moved index pattern guidelines to below input and removed some extra spacing

5. Indexing status screen: Indexing

6. Indexing status screen: Complete

Don't use EuiFormRow if you're not using form controls. Just use titles to add headings.

I also added the language="json" prop to the code blocks for code coloring. You may want to consider adding isCopyable in case users just want to copy and paste the whole block somewhere else.

7. Added position: relative to the panel housing the file upload forms

...so that the progress indicator is at the top of the panel not the flyout:

Before

After


Improvements I couldn't make on my own

a. When the file is first selected and is being parsed, it locks up the entire UI.

You can only see a sliver of the progress indicator as seen in the screenshot above. Consider finding a way to optimize so that the UI can still perform as the parsing is being done in the background.

b. When the indexing status is complete, add the words "Complete"

c. BUG (or feels like one): When trying to edit the pre-fab index pattern name

Anytime you try to type in the middle of the index pattern name it pushes you to the end of the string. Since you're creating this string for them, but not accounting for the specific patterns of lowercase, it makes editing only portions of the pre-fab name very frustrating.


Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@cchaos cchaos added [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.3.0 labels Jun 21, 2019
@cchaos cchaos requested review from nreese and kindsun June 21, 2019 10:30
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

lgtm
code review, tested in chrome

@cchaos
Copy link
Contributor Author

cchaos commented Jun 21, 2019

I had forgotten one more that I wanted to fix:

8. Change discard changes modal to a EuiConfirmModal

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@kindsun
Copy link
Contributor

kindsun commented Jun 25, 2019

I'm currently getting this error in the console on dragging & dropping a file. Looks like just a bad style selector?

image

@elasticmachine
Copy link
Contributor

💔 Build Failed

@nreese nreese requested a review from kindsun June 27, 2019 12:31
@elasticmachine
Copy link
Contributor

💔 Build Failed

@nreese
Copy link
Contributor

nreese commented Jun 27, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@kindsun kindsun left a comment

Choose a reason for hiding this comment

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

lgtm w/ green CI!

  • Code review
  • Run locally on Chrome

@kindsun
Copy link
Contributor

kindsun commented Jun 27, 2019

Thanks @cchaos, looks nice! Thanks @nreese for some final cleanup!

@elasticmachine
Copy link
Contributor

💔 Build Failed

@nreese
Copy link
Contributor

nreese commented Jun 27, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@nreese nreese merged commit 7988f23 into elastic:master Jun 27, 2019
nreese pushed a commit to nreese/kibana that referenced this pull request Jun 27, 2019
* Some fixes to the main listing screen

* Updates to file upload layouts

* Better discard changes modal

* Revert prettier auto-fixes in “Updates to file upload layouts"

* Revert prettier auto-fixes from "Some fixes to the main listing screen"

* Missed a line

* fix margin-bottom console warning

* clean up un-needed fileRef
@nreese
Copy link
Contributor

nreese commented Jun 27, 2019

7.x #39860

nreese added a commit that referenced this pull request Jun 28, 2019
* Some fixes to the main listing screen

* Updates to file upload layouts

* Better discard changes modal

* Revert prettier auto-fixes in “Updates to file upload layouts"

* Revert prettier auto-fixes from "Some fixes to the main listing screen"

* Missed a line

* fix margin-bottom console warning

* clean up un-needed fileRef
@cchaos cchaos deleted the maps-7.3-quick-fixes branch July 8, 2019 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:skip Skip the PR/issue when compiling release notes v7.3.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants