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] Add Unit Scale select #12183

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

dacook
Copy link
Member

@dacook dacook commented Feb 21, 2024

What? Why?

What should we test?

  • Visit ... page.

Release notes

Changelog Category (reviewers may add a label for the release notes):

  • User facing changes
  • API changes (V0, V1, DFC or Webhook)
  • Technical changes only
  • Feature toggled

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

@dacook dacook force-pushed the buu/unit-scale-12005 branch 2 times, most recently from e170c0a to 51d3ba3 Compare February 21, 2024 05:45
@dacook dacook self-assigned this Feb 21, 2024
@dacook dacook changed the title Add Unit Scale select [BUU] Add Unit Scale select Feb 22, 2024
describe "#scale_for_unit_value" do
describe "#variant_unit_options" do
let(:available_units) { "mg,g,kg,T,mL,cL,dL,L,kL,lb,oz,gal" }
subject { WeightsAndMeasures.variant_unit_options }
Copy link
Contributor

Choose a reason for hiding this comment

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

if subject(:instanciated_class) { WeightsAndMeasures.new(your_argument).variant_unit_options }
than the include will work

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

As I can guess, it is like a decorator. You might also consider leave this class untouched and create the decorator to deal with the decorated argument. Sounds good?

@dacook dacook force-pushed the buu/unit-scale-12005 branch 3 times, most recently from ef2fd4a to 5b649cd Compare February 29, 2024 22:36
dacook added 14 commits March 1, 2024 15:58
Table layout is tricky. I had originally hoped that the table would allow us to use min/max width. But that simply doesn't work with `table-layout: fixed`. So we need to set preconfigured widths.

Now I think that table layout doesn't bring any benefit, so I think we should consider switching to flexbox or grid. ButI'll wait until all elements are in place before trying anything new.
This is because it's going to move from product to variant soon, as part of Product Refactor.
This re-implements Angular JS function VariantUnitManager.variantUnitOptions()

Well.. almost. See next commit.
Who would have guessed it was this complicated.

Fingers crossed this doesn't break any other functionality...
A better way to arrange it, and as a bonus it makes the selectors simpler, yay!
I can't see any reason that fieldsets, which are containers, should share styles with inputs. Maybe font styles, but everything looks fine still.
This is more flexible and can find a field based on name, id or aria-label
The rem units are converted to em to make the padding relative to the chevron size. This means different font sizes will Just Work.
Thankfully I was able to use basic DOM features, so there's no coupling of the logic with tom-select.

It wasn't going to be simple to get tom-select to listen for the 'changed' class on the original select, so I found a simple solution with a CSS sibling selector instead.
but tomselect is intent on closing the dropdown and not letting me focus it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: All the things 💤
Development

Successfully merging this pull request may close these issues.

[BUU] Change unit scale
2 participants