Increase filters popover size and use full width in form components #352

galangel · 2021-05-13T12:12:13Z

Signed-off-by: galangel gal0angel@gmail.com

Description

The change resizes the modal to 600 ( instead of 400 ) and uses the full-width prop in the form components to take advantage of the space.
also Removed the unused styles

I explicitly left the range input at false because I think it looks better like this than with full width

Fullscreen at 100%

Issues Resolved

Filters popover modal is too small

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

Signed-off-by: galangel <gal0angel@gmail.com>

opensearch-ci-bot · 2021-05-13T12:39:00Z

✅ DCO Check Passed 3bfe0c3

seanneumann · 2021-05-13T19:31:03Z

I like this.

kgcreative · 2021-05-13T19:31:47Z

+1

ananzh · 2021-05-18T06:47:47Z

Unit test result:
Test Suites: 23 skipped, 1410 passed, 1410 of 1433 total
Tests: 256 skipped, 9 todo, 10341 passed, 10606 total
Snapshots: 2363 passed, 2363 total
Time: 43.678 s

Functional test result:

kavilla · 2021-05-18T19:17:24Z

Hey thanks for this! Tests pass for me. Desktop experience looks great!

But after talking with the team, I am worried about the mobile experience. As seen below, before and after on mobile. Even though Dashboards is intended to be a desktop experience, could there be improvements to make sure the text field isn't overflowing outside of the popover?

Before:

After:

JacobBrandt · 2021-05-18T23:05:12Z

For consistency, I would like to see screenshots 1, 3 and 4 have the same style rather than 4 be different for no reason. So since 1 and 3 are already the same why not make 4 the same style by adding a "Range" label above the range input and make range input full width.

JacobBrandt · 2021-05-18T23:11:43Z

src/plugins/data/public/ui/filter_bar/filter_editor/index.tsx

@@ -294,7 +295,6 @@ class FilterEditorUI extends Component<Props, State> {
          onChange={this.onFieldChange}
          singleSelection={{ asPlainText: true }}
          isClearable={false}
-          className="globalFilterEditor__fieldInput"


If you remove this class then the change you made to #355 is not going to work?

Yes, initially I used the subjects, now that I use the class, I Will revert this change in this PR

JacobBrandt · 2021-05-18T23:16:27Z

src/plugins/data/public/ui/filter_bar/filter_editor/range_value_input.tsx

@@ -84,6 +84,7 @@ function RangeValueInputUI(props: Props) {
  return (
    <div>
      <EuiFormControlLayoutDelimited
+        fullWidth={false}


As mentioned in my other comment I don't see why this value input is going to have a different style than phrases. I think instead you should change make this full width like you did the other input fields and change the aria-label below to just a normal label.

Thank you so much for taking a look!

About the full-width for the range:
I did so because, In my opinion, it wasn't looking great. I added the full-width prop so for consistency.
But sure, I will make a change and update the photo. Tell me what you think after.

About the label:
sadly that's a prop from EUI

If we're trying to get the same style then you will have to use similar components. Meaning switch the EuiFormControlLayoutDelimited to EuiFormRow like the other value inputs.

galangel · 2021-05-19T07:49:20Z

@kavilla Thank you for looking at this!
Didn't think about testing for mobile.
I Will check what can be done for the mobile experience and update the PR and any future PRs
Any guidelines for that? I assume we would like to support 400px min? (guessing from the current styling)

kgcreative · 2021-05-20T20:54:10Z

Hi @galangel -I don't think i've done a deep dive in terms of what the EUI sizes are for mobile devices. I know their 's' breakpoint is at 575px, and based on 2020 usage data, the most common min size is 375px (360px for iPhone mini, but i'm not sure that's something we want to use as a benchmark) - My sense is that below the "s" breakpoint, things should just go edge to edge and stacked.

Signed-off-by: galangel <gal0angel@gmail.com>

opensearch-ci-bot · 2021-05-31T11:40:23Z

❌ DCO Check Failed ac57170
Run ./dev-tools/signoff-check.sh remotes/origin/main ac571707a32f1fd5ab176ffa40c3f80d96180885 to check locally
Use git commit with -s to add 'Signed-of-by: {EMAIL}' on impacted commits

galangel · 2021-05-31T11:45:53Z

will now behave as in edit mode

galangel · 2021-05-31T11:53:59Z

@JacobBrandt can you re-review? I cant request in the UI =)

galangel · 2021-05-31T13:22:42Z

@kavilla I have updated the width settings

boktorbb · 2021-06-03T16:57:23Z

@galangel The DCO checks are failing. It looks like the merge commit wasn't properly signed

opensearch-ci-bot · 2021-06-06T15:02:33Z

❌ DCO Check Failed 69d19d0
Run ./dev-tools/signoff-check.sh remotes/origin/main 69d19d01f09a5652c641d6c29fe8381535d28a3c to check locally
Use git commit with -s to add 'Signed-of-by: {EMAIL}' on impacted commits

galangel · 2021-06-06T15:28:59Z

@boktorbb-amzn can you help me with that? I'm not sure how to do it =)

boktorbb · 2021-06-08T19:47:50Z

Hey @galangel, it looks like your new commit is signed but the DCO check fails because your older commit commit d3bc49304becfe7e80b2592bda27130d44c2654f is still not signed. You can do an interactive rebase and just drop that commit since it's just a merge commit. You can also amend the commit and sign it and rebase.

Signed-off-by: galangel <gal0angel@gmail.com>

opensearch-ci-bot · 2021-06-20T10:04:32Z

✅ DCO Check Passed df0d7bb

galangel · 2021-06-20T19:13:34Z

@boktorbb-amzn so I changed the commits, 🙏🏻 please review

boktorbb

Looks good to me. Thanks for the contribution!

galangel · 2021-06-27T09:03:17Z

Bumping =) @kavilla can you add your review so we could continue?

JacobBrandt · 2021-06-29T16:31:38Z

@galangel Just getting back from vacation. I'll test it out tonight.

kavilla · 2021-07-20T18:15:13Z

Sorry about the delay in merging this. Would you be able to create a backport PR into https://github.com/opensearch-project/OpenSearch-Dashboards/tree/1.x?

…ents * use full width * Update width * observer Backport PR: opensearch-project#352 Signed-off-by: galangel <gal0angel@gmail.com>

…ents (#672) * use full width * Update width * observer Backport PR: #352 Signed-off-by: galangel <gal0angel@gmail.com> Co-authored-by: Gal Angel <gal0angel@gmail.com>

use full width

3bfe0c3

Signed-off-by: galangel <gal0angel@gmail.com>

kavilla added the enhancement New feature or request label May 18, 2021

kavilla linked an issue May 18, 2021 that may be closed by this pull request

Filters popover modal is too small [BUG] #351

Closed

JacobBrandt suggested changes May 18, 2021

View reviewed changes

Update width

6fc4b89

Signed-off-by: galangel <gal0angel@gmail.com>

observer

df0d7bb

Signed-off-by: galangel <gal0angel@gmail.com>

galangel force-pushed the larger-filters-popover-size branch from 69d19d0 to df0d7bb Compare June 20, 2021 10:02

boktorbb approved these changes Jun 21, 2021

View reviewed changes

JacobBrandt approved these changes Jun 30, 2021

View reviewed changes

kavilla approved these changes Jun 30, 2021

View reviewed changes

ananzh added the v1.1.0 label Jul 7, 2021

kavilla merged commit 7604a49 into opensearch-project:main Jul 20, 2021

kavilla mentioned this pull request Jul 26, 2021

[1.x] Increase filters popover size and use full width in form components #672

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Increase filters popover size and use full width in form components #352

Increase filters popover size and use full width in form components #352

galangel commented May 13, 2021

opensearch-ci-bot commented May 13, 2021

seanneumann commented May 13, 2021

kgcreative commented May 13, 2021

ananzh commented May 18, 2021 •

edited

Loading

kavilla commented May 18, 2021

JacobBrandt commented May 18, 2021

JacobBrandt May 18, 2021

galangel May 19, 2021

JacobBrandt May 18, 2021

galangel May 19, 2021

JacobBrandt May 19, 2021

galangel commented May 19, 2021

kgcreative commented May 20, 2021

opensearch-ci-bot commented May 31, 2021

galangel commented May 31, 2021

galangel commented May 31, 2021

galangel commented May 31, 2021

boktorbb commented Jun 3, 2021 •

edited

Loading

opensearch-ci-bot commented Jun 6, 2021

galangel commented Jun 6, 2021

boktorbb commented Jun 8, 2021

opensearch-ci-bot commented Jun 20, 2021

galangel commented Jun 20, 2021

boktorbb left a comment

galangel commented Jun 27, 2021

JacobBrandt commented Jun 29, 2021

kavilla commented Jul 20, 2021 •

edited

Loading

Increase filters popover size and use full width in form components #352

Increase filters popover size and use full width in form components #352

Conversation

galangel commented May 13, 2021

Description

Issues Resolved

Check List

opensearch-ci-bot commented May 13, 2021

seanneumann commented May 13, 2021

kgcreative commented May 13, 2021

ananzh commented May 18, 2021 • edited Loading

kavilla commented May 18, 2021

JacobBrandt commented May 18, 2021

JacobBrandt May 18, 2021

Choose a reason for hiding this comment

galangel May 19, 2021

Choose a reason for hiding this comment

JacobBrandt May 18, 2021

Choose a reason for hiding this comment

galangel May 19, 2021

Choose a reason for hiding this comment

JacobBrandt May 19, 2021

Choose a reason for hiding this comment

galangel commented May 19, 2021

kgcreative commented May 20, 2021

opensearch-ci-bot commented May 31, 2021

galangel commented May 31, 2021

galangel commented May 31, 2021

galangel commented May 31, 2021

boktorbb commented Jun 3, 2021 • edited Loading

opensearch-ci-bot commented Jun 6, 2021

galangel commented Jun 6, 2021

boktorbb commented Jun 8, 2021

opensearch-ci-bot commented Jun 20, 2021

galangel commented Jun 20, 2021

boktorbb left a comment

Choose a reason for hiding this comment

galangel commented Jun 27, 2021

JacobBrandt commented Jun 29, 2021

kavilla commented Jul 20, 2021 • edited Loading

ananzh commented May 18, 2021 •

edited

Loading

boktorbb commented Jun 3, 2021 •

edited

Loading

kavilla commented Jul 20, 2021 •

edited

Loading