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.

Sign up for GitHub

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

Jump to bottom

[BUU] Fix Display ordering in shopfront field to allow re-ordering of the sequence #12860

Merged
merged 4 commits into from
Oct 4, 2024
Merged

[BUU] Fix Display ordering in shopfront field to allow re-ordering of the sequence #12860

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1545708
12852: remove fix height for the select2-choices
chahmedejaz Sep 14, 2024
052e3b6
12852: add remove choice icon for select2
chahmedejaz Sep 14, 2024
35f9c42
12852: remove unnecessary !important
chahmedejaz Sep 14, 2024
1550ca5
fix the choice drag issue
chahmedejaz Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions app/webpacker/css/admin_v3/components/select2.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ label .select2-container {
.select2-container {
.select2-choice,
.select2-choices {
height: $btn-relaxed-height !important; // !important is needed because of vendor/assets/stylesheets/select2.css.scss
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This rule was causing the choices to have a fixed height and hence the choices' height was not being adjusted as per the content in it.

Copy link
Member

@dacook dacook Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Great, I've always been against using height like this!
I recall that JB was using height to help line up elements in the new design, so it's possible that this will impact on the layout. I'll add a note for testers to check.

padding: 10px;
}
}
Expand Down Expand Up @@ -229,16 +228,14 @@ label .select2-container {
display: flex;
align-items: center;
justify-content: center;
padding-left: 7px;

.select2-search-choice-close {
Copy link
Collaborator Author

@chahmedejaz chahmedejaz Sep 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Stying the close button such that it's displayed with the icon-remove. Currently, it's not displayed.

Copy link
Member

Choose a reason for hiding this comment

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

Nice to add an improvement while we're here.
But perhaps there's a reason for this being hidden. I don't think the shop owners have the ability to remove categories. So do the removed ones will get repopulated at the end of the list after saving? (I guess that's what happens when a new category is added).

Testers will probably pick this up, but it's probably worth considering the impact of this a bit more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the note @dacook. This PR just tweaks the styling, and the actual functionality and behavior are the same as that of the old UI.
Just to be sure, I validated all the cases of updating the category i.e. add, remove. Both cases are working as expected. When we remove the category, it gets removed as well.
So, I think it's safe to say now the page is exactly mimicking the old UI with new styling.
Please let me know if you have any questions on this one. Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

Apologies, I misunderstood and thought that the remove buttons weren't there before, and that you were adding them.
But now I see that they were there already, and you were just fixing the display of them.
Screenshot 2024-09-17 at 9 19 20 AM

position: relative;
order: -1;
width: auto;
left: 0;
top: 0;
margin: 0;
padding: 0;
font-size: 85%;
@extend .icon-remove;
@extend [class^="icon-"], :before;
width: 0;
margin-left: 2px;
color: $color-1;
}
}
}
Expand Down
Loading