Skip to content

Commit 355fe1e

Browse files
committed
refactor
1 parent 74bd5b1 commit 355fe1e

File tree

2 files changed

+23
-73
lines changed

2 files changed

+23
-73
lines changed

ui/components/app/snaps/snap-ui-renderer/snap-ui-renderer.js

+22-58
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { memo, useCallback, useMemo, useRef, useState } from 'react';
1+
import React, { memo, useCallback, useMemo, useRef } from 'react';
22
import PropTypes from 'prop-types';
33
import { useSelector } from 'react-redux';
44
import { Container } from '@metamask/snaps-sdk/jsx';
@@ -81,66 +81,30 @@ const SnapUIRendererComponent = ({
8181

8282
const isProgrammaticScrollRef = useRef(false);
8383

84-
const [scrollState, setScrollState] = useState({
85-
buttonsEnabled: !requireScroll,
86-
showArrow: false,
87-
});
88-
89-
const handleScrollStateUpdate = useCallback(
90-
(buttonsAreEnabled, showArrow) => {
91-
if (
92-
buttonsAreEnabled !== scrollState.buttonsEnabled ||
93-
showArrow !== scrollState.showArrow
94-
) {
95-
setScrollState({
96-
buttonsEnabled: buttonsAreEnabled,
97-
showArrow,
98-
});
99-
}
100-
},
101-
[scrollState],
102-
);
103-
104-
const { scrollToBottom, ref } = useScrollRequired([], {
84+
const {
85+
scrollToBottom,
86+
ref,
87+
isScrolledToBottom,
88+
hasScrolledToBottom,
89+
isScrollable,
90+
onScroll,
91+
} = useScrollRequired([], {
10592
// Only enable the hook if we need to handle scroll events
10693
enabled: requireScroll,
107-
// This lets us batch state updates and avoid an unnecessary render
108-
onMeasure: ({ isScrollable: canScroll, hasMeasured }) => {
109-
if (!requireScroll || !hasMeasured) {
110-
return;
111-
}
112-
113-
handleScrollStateUpdate(!canScroll, canScroll);
114-
},
11594
});
11695

117-
const buttonsEnabled = useMemo(
118-
() => (requireScroll ? scrollState.buttonsEnabled : true),
119-
[requireScroll, scrollState.buttonsEnabled],
120-
);
96+
const shouldShowArrow = useMemo(() => {
97+
if (!requireScroll || isScrolledToBottom || hasScrolledToBottom) {
98+
return false;
99+
} else if (!isScrolledToBottom && !hasScrolledToBottom && isScrollable) {
100+
return true;
101+
}
102+
return false;
103+
}, [requireScroll, isScrolledToBottom, hasScrolledToBottom, isScrollable]);
121104

122-
const handleScroll = useCallback(
123-
(e) => {
124-
if (!requireScroll) {
125-
return;
126-
}
127-
128-
const isAtBottom =
129-
Math.abs(
130-
e.target.scrollTop + e.target.clientHeight - e.target.scrollHeight,
131-
) < 2;
132-
133-
handleScrollStateUpdate(
134-
scrollState.buttonsEnabled || isAtBottom,
135-
!isAtBottom && scrollState.showArrow,
136-
);
137-
},
138-
[
139-
requireScroll,
140-
scrollState.buttonsEnabled,
141-
handleScrollStateUpdate,
142-
scrollState.showArrow,
143-
],
105+
const buttonsEnabled = useMemo(
106+
() => (requireScroll ? !shouldShowArrow : true),
107+
[requireScroll, shouldShowArrow],
144108
);
145109

146110
const handleScrollToBottom = useCallback(() => {
@@ -200,15 +164,15 @@ const SnapUIRendererComponent = ({
200164
initialState={initialState}
201165
context={context}
202166
requireScroll={requireScroll}
203-
showArrow={scrollState.showArrow}
167+
showArrow={shouldShowArrow}
204168
buttonsEnabled={buttonsEnabled}
205169
scrollToBottom={handleScrollToBottom}
206170
>
207171
<Box
208172
className="snap-ui-renderer__content"
209173
data-testid="snap-ui-renderer__content"
210174
ref={ref}
211-
onScroll={handleScroll}
175+
onScroll={onScroll}
212176
height={BlockSize.Full}
213177
backgroundColor={backgroundColor}
214178
style={{

ui/hooks/useScrollRequired.js

+1-15
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,18 @@ import { usePrevious } from './usePrevious';
1212
* @param opt
1313
* @param {number} opt.offsetPxFromBottom
1414
* @param {boolean} opt.enabled
15-
* @param {Function} opt.onMeasure - A function to call when the scrollable content is measured. Useful for batching state updates.
1615
* @returns Flags for isScrollable and isScrollToBottom, a ref to use for the scrolling content, a scrollToBottom function and a onScroll handler.
1716
*/
1817
export const useScrollRequired = (
1918
dependencies = [],
20-
{ offsetPxFromBottom = 16, enabled = true, onMeasure } = {},
19+
{ offsetPxFromBottom = 16, enabled = true } = {},
2120
) => {
2221
const ref = useRef(null);
2322
const prevOffsetHeight = usePrevious(ref.current?.offsetHeight);
2423

2524
const [hasScrolledToBottomState, setHasScrolledToBottom] = useState(!enabled);
2625
const [isScrollableState, setIsScrollable] = useState(false);
2726
const [isScrolledToBottomState, setIsScrolledToBottom] = useState(!enabled);
28-
const [hasMeasured, setHasMeasured] = useState(!enabled);
2927

3028
const update = () => {
3129
if (!ref.current || !enabled) {
@@ -49,16 +47,6 @@ export const useScrollRequired = (
4947
setIsScrollable(isScrollable);
5048
}
5149

52-
if (!hasMeasured) {
53-
setHasMeasured(true);
54-
// Let's us batch state updates and avoid an unnecessary render
55-
// We can pass more variables to the onMeasure callback if needed
56-
onMeasure?.({
57-
isScrollable,
58-
hasMeasured: true, // Explicitly pass as true since hasMeasured is false at this point
59-
});
60-
}
61-
6250
setIsScrolledToBottom(!isScrollable || isScrolledToBottom);
6351

6452
if (!isScrollable || isScrolledToBottom) {
@@ -87,8 +75,6 @@ export const useScrollRequired = (
8775
}, [ref.current?.offsetHeight]);
8876

8977
const scrollToBottom = () => {
90-
// These variables aren't reliable during programmatic scrolls, since the action is async and the state is updated syncronously
91-
// we can get flickering states in the UI that depends on this hook.
9278
setIsScrolledToBottom(true);
9379
setHasScrolledToBottom(true);
9480

0 commit comments

Comments
 (0)