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

[Management] Highlight query in results for index pattern creation wizard #16529

Merged
merged 3 commits into from
Feb 8, 2018

Conversation

chrisronline
Copy link
Contributor

@chrisronline chrisronline commented Feb 5, 2018

Fixes #16297

16297

This PR fixes a visual regression where we used to highlight the query within the results in the table. For example, if you query for ki and one of the results is kibana, the ki part of kibana will appear bold.

screen shot 2018-02-05 at 1 33 20 pm

Other Fix

This PR also fixes an undocumented issue where typing in . will result in an error which isn't desired. That query is technically invalid, but there isn't a reason to show the same error as we show when the user typed in an illegal character. The fix for this warranted removing isIndexPatternQueryValid and replacing it with containsIllegalCharacters.

Previously:
screen shot 2018-02-05 at 1 35 55 pm

Now:
screen shot 2018-02-05 at 1 33 38 pm

Backporting

This needs to be backported to 6.2.1, but only part of the React code is in 6.2 so I'll just backport 920c59e and 2416dd8

/cc @cjcenizal

@elasticmachine
Copy link
Contributor

💔 Build Failed

@chrisronline
Copy link
Contributor Author

jenkin, test this

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@jen-huang jen-huang left a comment

Choose a reason for hiding this comment

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

The fixes are as promised, but I noticed some smaller UI issues. When there are illegal characters, the "Index Pattern" label and input field border are blue instead of red. The label becomes red if I click on it. The label and border also become red if I focus away from the page (to any other window), but returns to blue once I focus on the page again. See recording from Chrome:

feb-06-2018 22-18-37

@chrisronline
Copy link
Contributor Author

Nice catch @jen-huang!

It seems the focused state is overriding the error state. Is that intentional @snide or @cjcenizal ?

screen shot 2018-02-07 at 9 07 39 am

@cjcenizal
Copy link
Contributor

@chrisronline @jen-huang Yes, this is intentional. For accessibility, the focused state should override other states so you can always tell which part of the UI has focus.

@chrisronline
Copy link
Contributor Author

Thanks for clarifying @cjcenizal!

Copy link
Contributor

@jen-huang jen-huang left a comment

Choose a reason for hiding this comment

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

LGTM after the clarifications 😸

@jimgoodwin jimgoodwin added v6.2.2 and removed v6.2.1 labels Feb 8, 2018
@chrisronline chrisronline merged commit 32f50b0 into elastic:master Feb 8, 2018
@chrisronline chrisronline deleted the fix/16297 branch February 8, 2018 18:47
chrisronline added a commit to chrisronline/kibana that referenced this pull request Feb 8, 2018
…zard (elastic#16529)

* Highlight matches in the table

* Address issue with showing an error when the user types `.` which is invalid, but there shouldn't be an error

* Use strong
chrisronline added a commit that referenced this pull request Feb 8, 2018
…zard (#16529) (#16615)

* Highlight matches in the table

* Address issue with showing an error when the user types `.` which is invalid, but there shouldn't be an error

* Use strong
@chrisronline
Copy link
Contributor Author

Backport

6.x: 4b04332

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Index pattern management UI regression
6 participants