-
Notifications
You must be signed in to change notification settings - Fork 913
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
[MD] Address UX comments on index pattern stack #2611
Conversation
Looks like a snapshot failure? |
LGTM with one slight change - the table column header should not appear sentence case and should read Data Source Connection |
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
Codecov Report
@@ Coverage Diff @@
## main #2611 +/- ##
=======================================
Coverage 66.78% 66.79%
=======================================
Files 3207 3207
Lines 61174 61174
Branches 9329 9329
=======================================
+ Hits 40856 40860 +4
+ Misses 18083 18080 -3
+ Partials 2235 2234 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@kristenTian can you change the title to something meaningful? So when we squash and merge. The commit message makes sense for user. |
@@ -183,7 +183,7 @@ export const IndexPatternTable = ({ canSave, history }: Props) => { | |||
const columns = [ | |||
{ | |||
field: 'title', | |||
name: 'Pattern', | |||
name: 'Title', |
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.
Do we have insight or if this favorable or not?
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.
@KrooshalUX requested this change. She mentioned to be consistent with save object management table.
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.
@kristenTian @KrooshalUX - We discussed in the brainstorming meeting today - the "title" reference in the saved object table is a bit misleading, and "pattern" or "index pattern" is the more accurate term. One potential solution in the future would be to disambiguate the index pattern saved object title from the pattern itself: #2649
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.
Just blocking for more insight on the Pattern
to Title
change
Done I have included all changes the commit message already. So even with squash, it will be visible to user. |
@kavilla As we were working through all of the interfaces for MDS and working to aligning to established patterns, we determined that Index Pattern tables were inconsistent with the pattern across Dashboards, which is to use "Title", as it also reflects the object API. |
I would argue that this interface does not show a "title" or even a "pattern"; it is specifically an "index pattern". While creating an index pattern, OSD doesn't ask for a "title" or "pattern"; it asks the user for an "index pattern". PS, the index pattern creation page incorrectly calls it "Index pattern name" for a field that is really the "index pattern" and is not a name. We will have to fix that separately. edit: wrote this in a hurry and reading it out loud, i didn't like the tune; rewording it. |
This make sense to me, however still requires a further alignment on UX. Given the PR contains other small issues to address, I will revert the column title change for now and address it in a separate PR. |
We can move forward without the change to Pattern / Title - given the discussion around naming, APIs and table patterns that needs to happen at a larger scale. |
1a8268d
to
bc85910
Compare
- Update data source column header in index pattern table - Update index pattern column name to Title - Remove data source search field error check given it is not required Signed-off-by: Kristen Tian <[email protected]>
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
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-2611-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 189cf81a185f59a934d4d593206f7a115ebbd399
# Push it to GitHub
git push --set-upstream origin backport/backport-2611-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x Then, create a pull request where the |
…2611) - Update data source column header in index pattern table - Update index pattern column name to Title - Remove data source search field error check given it is not required Signed-off-by: Kristen Tian <[email protected]> Signed-off-by: Kristen Tian <[email protected]> (cherry picked from commit 189cf81)
…2611) - Update data source column header in index pattern table - Update index pattern column name to Title - Remove data source search field error check given it is not required Signed-off-by: Kristen Tian <[email protected]> Signed-off-by: Kristen Tian <[email protected]> (cherry picked from commit 189cf81) Signed-off-by: Su <[email protected]>
- Update data source column header in index pattern table - Update index pattern column name to Title - Remove data source search field error check given it is not required Signed-off-by: Kristen Tian <[email protected]> Signed-off-by: Kristen Tian <[email protected]> (cherry picked from commit 189cf81) Signed-off-by: Su <[email protected]> Signed-off-by: Kristen Tian <[email protected]> Signed-off-by: Su <[email protected]> Co-authored-by: Kristen Tian <[email protected]>
…2611) - Update data source column header in index pattern table - Update index pattern column name to Title - Remove data source search field error check given it is not required Signed-off-by: Kristen Tian <[email protected]> Signed-off-by: Kristen Tian <[email protected]> Signed-off-by: Sergey V. Osipov <[email protected]>
…2611) - Update data source column header in index pattern table - Update index pattern column name to Title - Remove data source search field error check given it is not required Signed-off-by: Kristen Tian <[email protected]> Signed-off-by: Kristen Tian <[email protected]> Signed-off-by: Arpit Bandejiya <[email protected]>
Signed-off-by: Kristen Tian [email protected]
Description
Check List
yarn test:jest
yarn test:jest_integration
yarn test:ftr