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

[BUG] fix suggestion list cutoff issue #2607

Merged
merged 2 commits into from
Oct 21, 2022

Conversation

ananzh
Copy link
Member

@ananzh ananzh commented Oct 18, 2022

Description

In this PR, we fixed the suggestion list cutoff in an easy way. We remove width 250px and add block display to allow browser to select a width and add ellipsis at the bottom.

However, we might have longer input that can't fit into the search. When we truncate the search, the elements look the same which might be still an issue for customer usage. We definitely need more feedbacks on the cutoff behavior.

Issue resolved:

#2555

Previous:
Screen Shot 2022-10-18 at 14 45 23

Current:
Screen Shot 2022-10-21 at 10 33 18

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

@ananzh ananzh added bug Something isn't working ux / ui Improvements or additions to user experience, flows, components, UI elements labels Oct 18, 2022
@ananzh ananzh self-assigned this Oct 18, 2022
@ananzh ananzh requested a review from a team as a code owner October 18, 2022 22:11
@ananzh ananzh force-pushed the fix-suggestion-cutoff branch from 84f5db8 to 4fa3e08 Compare October 18, 2022 22:13
@ananzh
Copy link
Member Author

ananzh commented Oct 18, 2022

This is a quick fix for the bug to improve current cx usage experience. This is not a permanent fix for suggestion list cutoff behavior.
We opened another follow-up issue for suggestion list cutoff behavior is opened here:
#2606

@ananzh ananzh added v2.4.0 'Issues and PRs related to version v2.4.0' backport 2.x labels Oct 18, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #2607 (84f5db8) into main (f29f734) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 84f5db8 differs from pull request most recent head 4fa3e08. Consider uploading reports for the commit 4fa3e08 to get more accurate results

@@            Coverage Diff             @@
##             main    #2607      +/-   ##
==========================================
- Coverage   66.81%   66.81%   -0.01%     
==========================================
  Files        3207     3207              
  Lines       61136    61136              
  Branches     9313     9313              
==========================================
- Hits        40851    40849       -2     
- Misses      18055    18056       +1     
- Partials     2230     2231       +1     
Impacted Files Coverage Δ
...ic/application/models/sense_editor/sense_editor.ts 64.00% <0.00%> (-0.89%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

joshuarrrr
joshuarrrr previously approved these changes Oct 19, 2022
CHANGELOG.md Outdated Show resolved Hide resolved
ashwin-pc
ashwin-pc previously approved these changes Oct 20, 2022
Copy link
Member

@ashwin-pc ashwin-pc left a comment

Choose a reason for hiding this comment

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

From the screenshot, longer strings that are cutoff still look okay to me. Suggestions arent expected to be complete and having a horizontal scrollbar in the suggestion window is not good ux.

@ananzh ananzh dismissed stale reviews from ashwin-pc and joshuarrrr via 3b0bc82 October 20, 2022 18:23
@ananzh ananzh force-pushed the fix-suggestion-cutoff branch from 3b0bc82 to 1ef5bc8 Compare October 20, 2022 18:29
@kavilla
Copy link
Member

kavilla commented Oct 20, 2022

From the screenshot, longer strings that are cutoff still look okay to me. Suggestions arent expected to be complete and having a horizontal scrollbar in the suggestion window is not good ux.

+1, also I wonder if this intentional behavior? Like if hover over and it shows the full string. Is there intention that we can add features here

@kgcreative
Copy link
Member

kgcreative commented Oct 21, 2022

From the screenshot, longer strings that are cutoff still look okay to me. Suggestions arent expected to be complete and having a horizontal scrollbar in the suggestion window is not good ux.

+1, also I wonder if this intentional behavior? Like if hover over and it shows the full string. Is there intention that we can add features here

I would +1 that this is an acceptable fix. Honestly, if there are features added, I would expect that they would automatically take up space on the left or right, so having the width of the container to be automatic with a max-width of 100% still makes sense to me. I think the only item I would add here is that it would be useful to see a truncation elipsis at the end of the cutoff, but that can be explored in a subsequent follow-up. (As far as I know these items are not selectable in their current form, click just loads the string into the search box and then users can select from there, so this feels like a preview that's intended to truncate, and somewhere a max-width got introduced that artificially constrains it.

Copy link
Member

@kgcreative kgcreative left a comment

Choose a reason for hiding this comment

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

tl;dr: LGTM 🚀

@ananzh ananzh force-pushed the fix-suggestion-cutoff branch from 1ef5bc8 to b106741 Compare October 21, 2022 16:53
In this PR, we fixed the suggestion list cutoff in an easy way.
We remove width 250px and add block display to allow browser to
select a width and add ellipsis at the bottom.

However, we might have longer input that can't fit into the search.
When we truncate the search, the elements look the same which might
be still an issue for customer usage. We definitely need more
feedbacks on the cutoff behavior.

Issue resolved:
opensearch-project#2555

Signed-off-by: Anan Zhuang <[email protected]>
@ananzh ananzh force-pushed the fix-suggestion-cutoff branch from b106741 to fb8b205 Compare October 21, 2022 17:37
@ananzh
Copy link
Member Author

ananzh commented Oct 21, 2022

I changed the fix to allow ellipsis shown at the bottom. Also update commit msg. Thanks for @AMoo-Miki 's idea!

Copy link
Member

@manasvinibs manasvinibs left a comment

Choose a reason for hiding this comment

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

LGTM

@ananzh ananzh merged commit 195cc8e into opensearch-project:main Oct 21, 2022
@opensearch-trigger-bot
Copy link
Contributor

The backport to 1.x failed:

The process '/usr/bin/git' failed with exit code 1

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-1.x 1.x
# Navigate to the new working tree
cd .worktrees/backport-1.x
# Create a new branch
git switch --create backport/backport-2607-to-1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 195cc8ef39e0399b50da8e48afd5cc7df20ac81f
# Push it to GitHub
git push --set-upstream origin backport/backport-2607-to-1.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.x

Then, create a pull request where the base branch is 1.x and the compare/head branch is backport/backport-2607-to-1.x.

opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 21, 2022
In this PR, we fixed the suggestion list cutoff in an easy way.
We remove width 250px and add block display to allow browser to
select a width and add ellipsis at the bottom.

However, we might have longer input that can't fit into the search.
When we truncate the search, the elements look the same which might
be still an issue for customer usage. We definitely need more
feedbacks on the cutoff behavior.

Issue resolved:
#2555

Signed-off-by: Anan Zhuang <[email protected]>

Signed-off-by: Anan Zhuang <[email protected]>
(cherry picked from commit 195cc8e)
ananzh added a commit to ananzh/OpenSearch-Dashboards that referenced this pull request Oct 21, 2022
In this PR, we fixed the suggestion list cutoff in an easy way.
We remove width 250px and add block display to allow browser to
select a width and add ellipsis at the bottom.

However, we might have longer input that can't fit into the search.
When we truncate the search, the elements look the same which might
be still an issue for customer usage. We definitely need more
feedbacks on the cutoff behavior.

Issue resolved:
opensearch-project#2555

Backport PR:
opensearch-project#2607

Signed-off-by: Anan Zhuang <[email protected]>
ananzh added a commit to ananzh/OpenSearch-Dashboards that referenced this pull request Oct 21, 2022
In this PR, we fixed the suggestion list cutoff in an easy way.
We remove width 250px and add block display to allow browser to
select a width and add ellipsis at the bottom.

However, we might have longer input that can't fit into the search.
When we truncate the search, the elements look the same which might
be still an issue for customer usage. We definitely need more
feedbacks on the cutoff behavior.

Issue resolved:
opensearch-project#2555

Backport PR:
opensearch-project#2607

Signed-off-by: Anan Zhuang <[email protected]>
joshuarrrr pushed a commit that referenced this pull request Oct 31, 2022
In this PR, we fixed the suggestion list cutoff in an easy way.
We remove width 250px and add block display to allow browser to
select a width and add ellipsis at the bottom.

However, we might have longer input that can't fit into the search.
When we truncate the search, the elements look the same which might
be still an issue for customer usage. We definitely need more
feedbacks on the cutoff behavior.

Issue resolved:
#2555

Backport PR:
#2607

Signed-off-by: Anan Zhuang <[email protected]>

Signed-off-by: Anan Zhuang <[email protected]>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 31, 2022
In this PR, we fixed the suggestion list cutoff in an easy way.
We remove width 250px and add block display to allow browser to
select a width and add ellipsis at the bottom.

However, we might have longer input that can't fit into the search.
When we truncate the search, the elements look the same which might
be still an issue for customer usage. We definitely need more
feedbacks on the cutoff behavior.

Issue resolved:
#2555

Backport PR:
#2607

Signed-off-by: Anan Zhuang <[email protected]>

Signed-off-by: Anan Zhuang <[email protected]>
(cherry picked from commit ba3e01f)
ananzh added a commit that referenced this pull request Nov 1, 2022
In this PR, we fixed the suggestion list cutoff in an easy way.
We remove width 250px and add block display to allow browser to
select a width and add ellipsis at the bottom.

However, we might have longer input that can't fit into the search.
When we truncate the search, the elements look the same which might
be still an issue for customer usage. We definitely need more
feedbacks on the cutoff behavior.

Issue resolved:
#2555

Signed-off-by: Anan Zhuang <[email protected]>

Signed-off-by: Anan Zhuang <[email protected]>
(cherry picked from commit 195cc8e)

Co-authored-by: Anan Zhuang <[email protected]>
joshuarrrr pushed a commit that referenced this pull request Nov 1, 2022
In this PR, we fixed the suggestion list cutoff in an easy way.
We remove width 250px and add block display to allow browser to
select a width and add ellipsis at the bottom.

However, we might have longer input that can't fit into the search.
When we truncate the search, the elements look the same which might
be still an issue for customer usage. We definitely need more
feedbacks on the cutoff behavior.

Issue resolved:
#2555

Backport PR:
#2607

Signed-off-by: Anan Zhuang <[email protected]>

Signed-off-by: Anan Zhuang <[email protected]>
(cherry picked from commit ba3e01f)

Co-authored-by: Anan Zhuang <[email protected]>
sipopo pushed a commit to sipopo/OpenSearch-Dashboards that referenced this pull request Dec 16, 2022
In this PR, we fixed the suggestion list cutoff in an easy way.
We remove width 250px and add block display to allow browser to
select a width and add ellipsis at the bottom.

However, we might have longer input that can't fit into the search.
When we truncate the search, the elements look the same which might
be still an issue for customer usage. We definitely need more
feedbacks on the cutoff behavior.

Issue resolved:
opensearch-project#2555

Signed-off-by: Anan Zhuang <[email protected]>

Signed-off-by: Anan Zhuang <[email protected]>
Signed-off-by: Sergey V. Osipov <[email protected]>
Arpit-Bandejiya pushed a commit to Arpit-Bandejiya/OpenSearch-Dashboards that referenced this pull request Jan 13, 2023
In this PR, we fixed the suggestion list cutoff in an easy way.
We remove width 250px and add block display to allow browser to
select a width and add ellipsis at the bottom.

However, we might have longer input that can't fit into the search.
When we truncate the search, the elements look the same which might
be still an issue for customer usage. We definitely need more
feedbacks on the cutoff behavior.

Issue resolved:
opensearch-project#2555

Signed-off-by: Anan Zhuang <[email protected]>

Signed-off-by: Anan Zhuang <[email protected]>
Signed-off-by: Arpit Bandejiya <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x bug Something isn't working ux / ui Improvements or additions to user experience, flows, components, UI elements v1.3.7 v2.4.0 'Issues and PRs related to version v2.4.0'
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants