fix(site): fix floating number on duration fields

[BrunoQuaresma]

Demo about the new <DurationField /> component.

Screen.Recording.2024-05-08.at.14.11.24.mov

Storybook on Chromatic: https://624de63c6aacee003aa84340-wnakkqswca.chromatic.com/?path=/story/components-durationfield--hours

[stirby]

Related: #13071

Assuming this will interpret units when the page loads? Just wondering how it looks if I change the value via CLI.

Can we add a tooltip when "Days" is greyed out explaining why it's blocked?

"Set duration to a multiple of 24 hours to change units."

[Parkreiner]

I think this looks good so far!
It actually reminds me of the component I built out for my Coder take-home, though obviously, using MUI simplifies the code a lot more compared to using HTML inputs

[Parkreiner]

@stirby Yeah, the way the component is set up right now is that it will default to days whenever the numeric value originally passed in is cleanly divisible by 24 hours. Otherwise, it uses hours

[BrunoQuaresma]

@Parkreiner thanks for the review. Did you have time to play around with it in the storybook? I'm going to create tests for it after being sure the UX is good enough.

[BrunoQuaresma]

@Parkreiner after implementing this component into the form I realized you were right about two things:

• It is better to only have the value as a number instead of having to deal with undefined.
• We need to handle external value updates to avoid a wrong state in the time unit.
  🙏

[BrunoQuaresma]

@Parkreiner I updated the code a bit and would love a second round of review.

@stirby I would appreciate it if you could QA the "Time until dormant" field experience and let me know if it is an improvement compared to what we have.

A quick demo:

Screen.Recording.2024-05-09.at.14.10.41.mov

[github-actions]

---

✔️ PR 13209 Updated successfully.
🚀 Access the credentials here.

cc: @BrunoQuaresma

[Parkreiner]

@BrunoQuaresma Have a couple of things to wrap up today, but at the very latest, I'll do a real round of review tomorrow morning

[Parkreiner]

I think this looks really good!
I don't know how much more you need to update the code to get the failing tests to pass, but if you think they just need a few tiny tweaks, I can go ahead and approve, just to make sure you're not blocked

[Parkreiner]

Okay, cool – approving

[stirby]

@BrunoQuaresma, few things.

I like the improved error tooltip, but it seems to appear only under certain conditions:

Screen.Recording.2024-05-10.at.11.58.41.AM.mov

It seems like if I enter an incorrect value right after switching units, the error appears. I'm also able to sometimes apply the float values, but it uses the integer.

Can we change the conditional to be more consistent? IE showing up any time there's an incorrect value?

Also:

• Should we make the entry box fill the width like the old one?
• Could we apply it to the other options?

Looks great otherwise, nice work.

[BrunoQuaresma]

@stirby I would appreciate another QA round. I updated the preview with the latest changes 🙏

[stirby]

Looks good, feels good. Did you remove the ability to enter decimals? I think it's a clean solution.

Would ask that @Parkreiner takes one more look.

[Parkreiner]

@stirby Yeah, I can do another once-over in about an hour

[Parkreiner]

Yup, I think the component's in a good spot now

[Parkreiner]

Was there a reason why props.error wasn't destructured from the props at the top of the component?

[BrunoQuaresma]

Because it is used by the TextField props as well.

[Parkreiner]

Just making sure: this is just a precaution/double-bookkeeping, right? Even though the UI has the disabled check to prevent the days unit from being selected at certain points, the reducer is also enforcing that the state update can't go through, in case the UI is set up wrong?

[BrunoQuaresma]

Yes, I think it can be useful to have it in the reducer + disabled attribute. Wdyt?