Skip to content

Commit

Permalink
fix(form): Added fixes required for Concurrent Rendering
Browse files Browse the repository at this point in the history
  • Loading branch information
mlaursen committed Nov 27, 2021
1 parent ff8a1d6 commit b4994f4
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 47 deletions.
16 changes: 13 additions & 3 deletions packages/form/src/select/Listbox.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { forwardRef, HTMLAttributes, useCallback, useRef } from "react";
import {
forwardRef,
HTMLAttributes,
useCallback,
useEffect,
useRef,
} from "react";
import cn from "classnames";
import { List, ListElement } from "@react-md/list";
import {
Expand Down Expand Up @@ -360,14 +366,18 @@ export const Listbox = forwardRef<ListElement, ListboxProps>(function Listbox(
});

const prevVisible = useRef(visible);
if (visible !== prevVisible.current) {
useEffect(() => {
if (prevVisible.current === visible) {
return;
}

prevVisible.current = visible;
// whenever it gains visibility, try to set the focused index to the
// current active value
if (visible) {
setFocusedIndex(getIndex());
}
}
}, [getIndex, setFocusedIndex, visible]);

const handleFocus = useCallback(
(event: React.FocusEvent<ListElement>) => {
Expand Down
12 changes: 7 additions & 5 deletions packages/form/src/slider/useDiscreteValueVisibility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ export function useDiscreteValueVisibility({
// immediately
const [isModeTransition, setModeTransition] = useState(false);
const [visible, setVisible] = useState(false);
if (discrete && visible && disabled) {
setVisible(false);
}

useEffect(() => {
if (!discrete) {
Expand All @@ -83,6 +80,11 @@ export function useDiscreteValueVisibility({
return;
}

if (discrete && visible && disabled) {
setVisible(false);
return;
}

if (!isKeyboard) {
// only considered a "transition" when the tooltip is already visible and
// switching away from keyboard mode
Expand All @@ -96,8 +98,8 @@ export function useDiscreteValueVisibility({
// the user switch between the modes more easily so if the active element is
// the current thumb, we're good to go
setModeTransition(false);
setVisible(document.activeElement === ref.current);
}, [isKeyboard, visible, discrete]);
setVisible(!disabled && document.activeElement === ref.current);
}, [isKeyboard, visible, discrete, disabled]);

useEffect(() => {
if (!discrete) {
Expand Down
38 changes: 20 additions & 18 deletions packages/form/src/slider/useRangeSlider.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useMemo, useRef, useState } from "react";
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { nearest } from "@react-md/utils";

import {
Expand Down Expand Up @@ -197,23 +197,25 @@ export function useRangeSlider(
}, [onChange, value]);

const prev = useRef({ min, max, step });
if (
prev.current.min !== min ||
prev.current.max !== max ||
prev.current.step !== step
) {
// ensure that if the `min`, `max`, or `step` value changes that the value
// is updated as well. Without this, there will be a runtime error if the
// value is not within the new range.
prev.current = { min, max, step };
const steps = getSteps(min, max, step);
const nextValue: RangeSliderValue = [
nearest(value[0], min, max, steps),
nearest(value[1], min, max, steps),
];
currentValue.current = nextValue;
setValue(nextValue);
}
useEffect(() => {
if (
prev.current.min !== min ||
prev.current.max !== max ||
prev.current.step !== step
) {
// ensure that if the `min`, `max`, or `step` value changes that the value
// is updated as well. Without this, there will be a runtime error if the
// value is not within the new range.
prev.current = { min, max, step };
const steps = getSteps(min, max, step);
const nextValue: RangeSliderValue = [
nearest(value[0], min, max, steps),
nearest(value[1], min, max, steps),
];
currentValue.current = nextValue;
setValue(nextValue);
}
}, [min, max, step, value]);

if (updateOn === "change" && currentValue.current !== value) {
currentValue.current = value;
Expand Down
30 changes: 16 additions & 14 deletions packages/form/src/slider/useSlider.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useMemo, useRef, useState } from "react";
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { nearest } from "@react-md/utils";

import {
Expand Down Expand Up @@ -116,19 +116,21 @@ export function useSlider(
}, [onChange, value]);

const prev = useRef({ min, max, step });
if (
prev.current.min !== min ||
prev.current.max !== max ||
prev.current.step !== step
) {
// ensure that if the `min`, `max`, or `step` value changes that the value
// is updated as well. Without this, there will be a runtime error if the
// value is not within the new range.
prev.current = { min, max, step };
const nextValue = nearest(value, min, max, getSteps(min, max, step));
currentValue.current = nextValue;
setValue(nextValue);
}
useEffect(() => {
if (
prev.current.min !== min ||
prev.current.max !== max ||
prev.current.step !== step
) {
// ensure that if the `min`, `max`, or `step` value changes that the value
// is updated as well. Without this, there will be a runtime error if the
// value is not within the new range.
prev.current = { min, max, step };
const nextValue = nearest(value, min, max, getSteps(min, max, step));
currentValue.current = nextValue;
setValue(nextValue);
}
}, [min, max, step, value]);

if (updateOn === "change" && currentValue.current !== value) {
currentValue.current = value;
Expand Down
2 changes: 2 additions & 0 deletions packages/form/src/slider/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ export const getDragPercentage = ({
min,
max,
value: thumb1Value,
validate: false,
});

let thumb2Percentage: number | undefined;
Expand All @@ -260,6 +261,7 @@ export const getDragPercentage = ({
min,
max,
value: thumb2Value,
validate: false,
});
thumb1Percentage = Math.min(thumb1Percentage, percentage);
thumb2Percentage =
Expand Down
9 changes: 6 additions & 3 deletions packages/form/src/text-field/TextArea.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
Ref,
TextareaHTMLAttributes,
useCallback,
useEffect,
useRef,
useState,
} from "react";
Expand Down Expand Up @@ -171,9 +172,11 @@ export const TextArea = forwardRef<HTMLTextAreaElement, TextAreaProps>(
});

const [height, setHeight] = useState<number>();
if (resize !== "auto" && typeof height === "number") {
setHeight(undefined);
}
useEffect(() => {
if (resize !== "auto" && typeof height === "number") {
setHeight(undefined);
}
}, [resize, height]);

const maskRef = useRef<HTMLTextAreaElement | null>(null);
const [scrollable, setScrollable] = useState(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ exports[`TextArea should handle updating the height correctly based on the resiz
data-testid="container"
id="text-area"
rows="2"
style=""
/>
</div>
`;
11 changes: 7 additions & 4 deletions packages/form/src/useFieldStates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
FocusEvent,
FocusEventHandler,
useCallback,
useEffect,
useRef,
useState,
} from "react";
Expand Down Expand Up @@ -129,10 +130,12 @@ export function useFieldStates<E extends FormElement>({
// `useNumberField` hook since the `value` will be set back to the empty
// string on invalid numbers.
const prevValue = useRef(value);
if (prevValue.current !== value && typeof value === "string") {
prevValue.current = value;
setValued(value.length > 0);
}
useEffect(() => {
if (prevValue.current !== value && typeof value === "string") {
prevValue.current = value;
setValued(value.length > 0);
}
}, [value]);

return {
valued,
Expand Down

0 comments on commit b4994f4

Please sign in to comment.