Skip to content

fix(site): fix floating number on duration fields #13209

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

Merged
merged 18 commits into from
May 17, 2024
Merged

fix(site): fix floating number on duration fields #13209

Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
6d359fd
Add DurationField component
BrunoQuaresma May 8, 2024
5ac0602
Add empty story
BrunoQuaresma May 8, 2024
95e4f44
Avoid negative values
BrunoQuaresma May 8, 2024
4a48102
Use duration field time_til_dormant_ms
BrunoQuaresma May 9, 2024
09ddb8f
Fix parent updates
BrunoQuaresma May 9, 2024
3427160
Fix helper text
BrunoQuaresma May 9, 2024
eef159c
Support errors
BrunoQuaresma May 9, 2024
b3042c6
Use valueMs to make the value is in miliseconds
BrunoQuaresma May 10, 2024
e6101cf
Replace useState by useReducer
BrunoQuaresma May 10, 2024
c82fc8f
Add 10 to base int
BrunoQuaresma May 10, 2024
1b77488
Merge branch 'main' of https://github.com/coder/coder into bq/fix-flo…
BrunoQuaresma May 13, 2024
9036465
Use a number mask
BrunoQuaresma May 13, 2024
cac3a89
Make number input full width
BrunoQuaresma May 13, 2024
d89ec94
Add duration field to auto deletion
BrunoQuaresma May 13, 2024
0a93c9f
Apply duration field to failure clean up
BrunoQuaresma May 13, 2024
5a35e1a
Merge branch 'main' of https://github.com/coder/coder into bq/fix-flo…
BrunoQuaresma May 17, 2024
45a5eb5
Add extra tests to storybook
BrunoQuaresma May 17, 2024
a6ff426
Fix tests
BrunoQuaresma May 17, 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
Prev Previous commit
Next Next commit
Replace useState by useReducer
  • Loading branch information
@BrunoQuaresma
BrunoQuaresma committed May 10, 2024
commit e6101cf18f77cf04ac3371ed3cfb07db36f12d37
77 changes: 54 additions & 23 deletions site/src/components/DurationField/DurationField.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import FormHelperText from "@mui/material/FormHelperText";
import MenuItem from "@mui/material/MenuItem";
import Select from "@mui/material/Select";
import TextField, { type TextFieldProps } from "@mui/material/TextField";
import { useState, type FC, useEffect } from "react";
import { type FC, useEffect, useReducer } from "react";
import {
type TimeUnit,
durationInDays,
Expand All @@ -23,19 +23,62 @@ type State = {
durationFieldValue: string;
};

type Action =
| { type: "SYNC_WITH_PARENT"; parentValueMs: number }
| { type: "CHANGE_DURATION_FIELD_VALUE"; fieldValue: string }
| { type: "CHANGE_TIME_UNIT"; unit: TimeUnit };

const reducer = (state: State, action: Action): State => {
switch (action.type) {
case "SYNC_WITH_PARENT": {
return initState(action.parentValueMs);
}
case "CHANGE_DURATION_FIELD_VALUE": {
return {
...state,
durationFieldValue: action.fieldValue,
};
}
case "CHANGE_TIME_UNIT": {
const currentDurationMs = durationInMs(
state.durationFieldValue,
state.unit,
);

if (
action.unit === "days" &&
!canConvertDurationToDays(currentDurationMs)
) {
return state;
}
Comment on lines +48 to +53
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator Author

@BrunoQuaresma BrunoQuaresma May 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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


return {
unit: action.unit,
durationFieldValue:
action.unit === "hours"
? durationInHours(currentDurationMs).toString()
: durationInDays(currentDurationMs).toString(),
};
}
default: {
return state;
}
}
};

export const DurationField: FC<DurationFieldProps> = (props) => {
const {
valueMs: parentValueMs,
onChange,
helperText,
...textFieldProps
} = props;
const [state, setState] = useState<State>(() => initState(parentValueMs));
const [state, dispatch] = useReducer(reducer, initState(parentValueMs));
const currentDurationMs = durationInMs(state.durationFieldValue, state.unit);

useEffect(() => {
if (parentValueMs !== currentDurationMs) {
setState(initState(parentValueMs));
dispatch({ type: "SYNC_WITH_PARENT", parentValueMs });
}
}, [currentDurationMs, parentValueMs]);

Expand All @@ -55,10 +98,10 @@ export const DurationField: FC<DurationFieldProps> = (props) => {
onChange={(e) => {
const durationFieldValue = e.currentTarget.value;

setState((state) => ({
...state,
durationFieldValue,
}));
dispatch({
type: "CHANGE_DURATION_FIELD_VALUE",
fieldValue: durationFieldValue,
});

const newDurationInMs = durationInMs(
durationFieldValue,
Expand All @@ -78,23 +121,15 @@ export const DurationField: FC<DurationFieldProps> = (props) => {
value={state.unit}
onChange={(e) => {
const unit = e.target.value as TimeUnit;
setState(() => ({
dispatch({
type: "CHANGE_TIME_UNIT",
unit,
durationFieldValue:
unit === "hours"
? durationInHours(currentDurationMs).toString()
: durationInDays(currentDurationMs).toString(),
}));
});
}}
inputProps={{ "aria-label": "Time unit" }}
IconComponent={KeyboardArrowDown}
>
<MenuItem
value="hours"
disabled={!canConvertDurationToHours(currentDurationMs)}
>
Hours
</MenuItem>
<MenuItem value="hours">Hours</MenuItem>
<MenuItem
value="days"
disabled={!canConvertDurationToDays(currentDurationMs)}
Expand Down Expand Up @@ -147,7 +182,3 @@ function daysToDuration(days: number): number {
function canConvertDurationToDays(duration: number): boolean {
return Number.isInteger(durationInDays(duration));
}

function canConvertDurationToHours(duration: number): boolean {
return Number.isInteger(durationInHours(duration));
}