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

[BUU] Dropdown UI tweaks (tom-select) #11932

Merged

Conversation

dacook
Copy link
Member

@dacook dacook commented Dec 12, 2023

What? Why?

What should we test?

With admin_style_v3 feature toggle,
Check that dropdowns behave as expected, eg in:

  • /admin/products
  • /admin/orders
  • /admin/enterprises/*/edit#/address_panel

Note that some dropdowns still have the older style, they can be changed over in future PRs

Single select with search:

Screen Shot 2023-12-15 at 3 50 21 pm

Single select with no search:

Screen Shot 2023-12-15 at 3 50 37 pm

@dacook dacook added pr-no-test technical changes only These pull requests do not contain user facing changes and are grouped in release notes labels Dec 12, 2023
@dacook

This comment was marked as resolved.

@mariocarabotta mariocarabotta added the pr-staged-au staging.openfoodnetwork.org.au label Dec 13, 2023
We should be able to use @extend .icon-chevron-down, but I couldn't get it to work. I'd like to have a better method for this, but we should upgrade our ancient FontAwesome before worrying about that.
The query input can grow to fill the space.
Hmm, but this isn't useful until we get Tom-Select to work the way we want..

To do that, I think we'd ned to hook into TS to clear the current selection when focused, then set it back upon blur (if no selection was made). Hmm, but we still want it to show slected in the dropdown list.
Can we do it with css maybe?
@dacook dacook added pr-staged-au staging.openfoodnetwork.org.au and removed pr-staged-au staging.openfoodnetwork.org.au labels Dec 14, 2023
It's more compact, and we don't need to see the currently selected value because it's highlighted in the list already.
This is unrelated to the rest of the PR, I just noticed this issue so decided to fix it. I can't find any explanation, or think of any good reason for this rule, so I'm burning it.
@dacook dacook marked this pull request as ready for review December 15, 2023 04:48
@mariocarabotta mariocarabotta added pr-staged-au staging.openfoodnetwork.org.au and removed pr-staged-au staging.openfoodnetwork.org.au labels Dec 17, 2023
@mariocarabotta

This comment was marked as resolved.

Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

Great!

@mariocarabotta mariocarabotta added pr-staged-au staging.openfoodnetwork.org.au and removed pr-staged-au staging.openfoodnetwork.org.au labels Dec 19, 2023
@sigmundpetersen sigmundpetersen removed the pr-staged-au staging.openfoodnetwork.org.au label Dec 20, 2023
@dacook
Copy link
Member Author

dacook commented Dec 28, 2023

  • i'd be good if when highlighted, this uses the Orient colour border
  • there's a strange double shadow showing when selected, see screenshot above.

We found that this only seems to affect Mario's browser (Brave), so won't try and fix it at this stage.

Copy link
Collaborator

@rioug rioug left a comment

Choose a reason for hiding this comment

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

Nice one 👏 I just have a couple of comments

app/views/admin/products_v3/_sort.html.haml Outdated Show resolved Hide resolved
app/webpacker/css/admin_v3/components/tom_select.scss Outdated Show resolved Hide resolved
@mariocarabotta mariocarabotta added pr-staged-au staging.openfoodnetwork.org.au and removed pr-staged-au staging.openfoodnetwork.org.au labels Jan 4, 2024
@dacook dacook requested a review from rioug January 9, 2024 05:26
@mariocarabotta mariocarabotta added pr-staged-au staging.openfoodnetwork.org.au and removed pr-staged-au staging.openfoodnetwork.org.au labels Jan 10, 2024
@mariocarabotta
Copy link
Collaborator

  • i'd be good if when highlighted, this uses the Orient colour border
  • there's a strange double shadow showing when selected, see screenshot above.

We found that this only seems to affect Mario's browser (Brave), so won't try and fix it at this stage.

this was bugging me too much ;D

I think it can be fixed by changing vertical padding from 0.5rem to 0.6 rem here

.plugin-dropdown_input .dropdown-input {
background-color: #fff;
border: 1px solid #f8f9fa;
box-shadow: none;
padding: 0.6rem 0.75rem;
}

Screenshot 2024-01-10 at 10 50 47

Copy link
Collaborator

@rioug rioug left a comment

Choose a reason for hiding this comment

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

Nice 1

I'm not sure why this requires extra padding here, but it looks good 🤷

Co-authored-by: Mario Carabotta <6696729+mariocarabotta@users.noreply.github.com>
@dacook dacook added the pr-staged-au staging.openfoodnetwork.org.au label Jan 10, 2024
@dacook
Copy link
Member Author

dacook commented Jan 10, 2024

Thanks Mario, that looks good to me. I've updated it, and started deploying to au_staging

@dacook dacook removed pr-no-test pr-staged-au staging.openfoodnetwork.org.au labels Jan 11, 2024
@dacook
Copy link
Member Author

dacook commented Jan 11, 2024

As this is quite a different component now, and it affects multiple screens, I'm going to request some testing.

@RachL RachL added pr-staged-fr staging.coopcircuits.fr and removed technical changes only These pull requests do not contain user facing changes and are grouped in release notes labels Jan 11, 2024
@RachL
Copy link
Contributor

RachL commented Jan 11, 2024

Note that some dropdowns still have the older style, they can be changed over in future PRs

@dacook just to be sure, when speaking about "old style" you include placeholder, size and "All" option?

@dacook
Copy link
Member Author

dacook commented Jan 11, 2024

when speaking about "old style" you include placeholder, size and "All" option?

More precisely, some dropdowns use the old "select2" plugin, which I have not restyled yet. These appear with a triangle icon, and a separate search input like this:
Screenshot 2024-01-12 at 8 15 15 am

(The new dropdowns use the "tom-select" plugin)

@dacook dacook added the feature toggled These pull requests' changes are invisible by default and are grouped in release notes label Jan 11, 2024
@sigmundpetersen
Copy link
Contributor

Hey @RachL are you choosing the rebase option when updating the branch?
image

@RachL
Copy link
Contributor

RachL commented Jan 12, 2024

@sigmundpetersen yes I thought I did, but does not look like it :/

@filipefurtad0
Copy link
Contributor

Merging and including it in the release, as per this discussion.

@filipefurtad0 filipefurtad0 merged commit 4c96eb4 into openfoodfoundation:master Jan 12, 2024
50 checks passed
@RachL RachL removed the pr-staged-fr staging.coopcircuits.fr label Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature toggled These pull requests' changes are invisible by default and are grouped in release notes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Dropdown UI tweaks
7 participants