Skip to content

Commit

Permalink
Retain some context in comments
Browse files Browse the repository at this point in the history
  • Loading branch information
tyxla committed Oct 24, 2024
1 parent 8fdcc76 commit 544528b
Show file tree
Hide file tree
Showing 26 changed files with 61 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ export function getAutoCompleterUI( autocompleter ) {
} else if ( isVisible && text.length === 0 ) {
startAnimation( false );
}
// We want to avoid introducing unexpected side effects.
// See https://github.com/WordPress/gutenberg/pull/41820
}, [ items, isVisible, text ] );

const activeItemStyles = usePreferredColorSchemeStyle(
Expand Down Expand Up @@ -109,6 +111,8 @@ export function getAutoCompleterUI( autocompleter ) {
}
} );
},
// We want to avoid introducing unexpected side effects.
// See https://github.com/WordPress/gutenberg/pull/41820
[ isVisible ]
);

Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/autocomplete/autocompleter-ui.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ export function getAutoCompleterUI( autocompleter: WPCompleter ) {
useLayoutEffect( () => {
onChangeOptions( items );
announce( items );
// We want to avoid introducing unexpected side effects.
// See https://github.com/WordPress/gutenberg/pull/41820
}, [ items ] );

if ( items.length === 0 ) {
Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/autocomplete/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,8 @@ export function useAutocomplete( {
: AutocompleterUI
);
setFilterValue( query === null ? '' : query );
// We want to avoid introducing unexpected side effects.
// See https://github.com/WordPress/gutenberg/pull/41820
}, [ textContent ] );

const { key: selectedKey = '' } = filteredOptions[ selectedIndex ] || {};
Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/color-palette/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ function ColorPalette( {
scrollViewRef.current.scrollTo( { x: 0, y: 0 } );
}
}
// Not adding additional dependencies until the component can be refactored and updated safely.
// Please see https://github.com/WordPress/gutenberg/pull/41253 for discussion and details.
}, [ currentSegment ] );

function isSelectedCustom() {
Expand Down
4 changes: 4 additions & 0 deletions packages/components/src/color-picker/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ function ColorPicker( {
if ( onHandleHardwareButtonPress ) {
onHandleHardwareButtonPress( onButtonPress );
}
// TODO: Revisit this to discover if there's a good reason for omitting
// the hook’s dependencies and running it a single time. Ideally there
// may be a way to refactor and obviate the disabled lint rule. If not,
// this comment should be replaced by one that explains the reasoning.
}, [] );

function onButtonPress( action ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ const BottomSheetNavigationScreen = ( {
* callbacks triggered based upon which screen is currently active.
*
* Related: https://github.com/WordPress/gutenberg/pull/36328#discussion_r768897546
*
* Also see https://github.com/WordPress/gutenberg/pull/41166.
*/
}, [] )
);
Expand Down Expand Up @@ -128,6 +130,7 @@ const BottomSheetNavigationScreen = ( {
</TouchableHighlight>
</ScrollView>
);
// See https://github.com/WordPress/gutenberg/pull/41166
}, [
children,
isFocused,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const BottomSheetSubSheet = ( {
if ( showSheet ) {
setIsFullScreen( isFullScreen );
}
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ showSheet, isFullScreen ] );

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const ColorSettingsMemo = memo(
useEffect( () => {
shouldEnableBottomSheetMaxHeight( true );
onHandleClosingBottomSheet( null );
// See https://github.com/WordPress/gutenberg/pull/41166
}, [] );
return (
<BottomSheet.NavigationContainer>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const PickerScreen = () => {
onHandleHardwareButtonPress={ onHandleHardwareButtonPress }
/>
);
// See https://github.com/WordPress/gutenberg/pull/41166
}, [
setColor,
currentValue,
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/mobile/image/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ const ImageComponent = ( {
}
}
return () => ( isCurrent = false );
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ url ] );

const onContainerLayout = ( event ) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export const KeyboardAvoidingView = ( {
keyboardShowSubscription.remove();
keyboardHideSubscription.remove();
};
// See https://github.com/WordPress/gutenberg/pull/41166
}, [] );

function onSafeAreaInsetsUpdate( { safeAreaInsets } ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ export default function LinkPickerResults( {
return {
fetchMoreSuggestions: debounce( fetchMore, REQUEST_DEBOUNCE_DELAY ),
};
// Not adding dependencies for now, to avoid introducing a regression
// (see https://github.com/WordPress/gutenberg/pull/23922#discussion_r1170634879).
}, [] );

// Prevent setting state when unmounted.
Expand All @@ -87,6 +89,7 @@ export default function LinkPickerResults( {
setHasAllSuggestions( false );
setLinks( [ directEntry ] );
fetchMoreSuggestions( { query, links: [ directEntry ] } );
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ query ] );

const onEndReached = () => fetchMoreSuggestions( { query, links } );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const LinkPickerScreen = ( { returnScreenName } ) => {
onCancel={ onCancel }
/>
);
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ inputValue ] );
};

Expand Down
6 changes: 6 additions & 0 deletions packages/components/src/mobile/link-settings/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ function LinkSettings( {
if ( onHandleClosingBottomSheet ) {
onHandleClosingBottomSheet( onCloseSettingsSheet );
}
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ urlInputValue, labelInputValue, linkRelInputValue ] );

useEffect( () => {
Expand All @@ -112,6 +113,7 @@ function LinkSettings( {
if ( url !== urlInputValue ) {
setUrlInputValue( url || '' );
}
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ url ] );

useEffect( () => {
Expand All @@ -135,6 +137,7 @@ function LinkSettings( {
if ( prevEditorSidebarOpened && ! editorSidebarOpened ) {
onSetAttributes();
}
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ editorSidebarOpened, isVisible ] );

useEffect( () => {
Expand All @@ -147,6 +150,7 @@ function LinkSettings( {
url: prependHTTP( urlValue ),
} );
}
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ urlValue ] );

const onChangeURL = useCallback(
Expand Down Expand Up @@ -176,6 +180,7 @@ function LinkSettings( {
rel: linkRelInputValue,
} );
}
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ urlInputValue, labelInputValue, linkRelInputValue, setAttributes ] );

const onCloseSettingsSheet = useCallback( () => {
Expand Down Expand Up @@ -208,6 +213,7 @@ function LinkSettings( {
rel: updatedRel,
} );
},
// See https://github.com/WordPress/gutenberg/pull/41166
[ linkRelInputValue ]
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const LinkSettingsScreen = ( props ) => {
urlValue={ inputValue }
/>
);
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ props, inputValue, navigation, route ] );
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,14 @@ const SegmentedControls = ( {
useEffect( () => {
setActiveSegmentIndex( selectedSegmentIndex );
segmentHandler( segments[ selectedSegmentIndex ] );
// See https://github.com/WordPress/gutenberg/pull/41166
}, [] );

useEffect( () => {
positionAnimationValue.setValue(
calculateEndValue( activeSegmentIndex )
);
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ segmentsDimensions ] );

const containerStyle = usePreferredColorSchemeStyle(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const useConvertUnitToMobile = ( value, unit, styles ) => {
return () => {
dimensionsChangeSubscription.remove();
};
// See https://github.com/WordPress/gutenberg/pull/41166
}, [] );

const onDimensionsChange = useCallback( ( { window } ) => {
Expand All @@ -82,6 +83,7 @@ const useConvertUnitToMobile = ( value, unit, styles ) => {
valueToConvert,
valueUnit
);
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ windowSizes, value, unit ] );
};

Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/navigation/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export function Navigation( {
if ( activeMenu !== menu ) {
setActiveMenu( activeMenu );
}
// Ignore exhaustive-deps here, as it would require either a larger refactor or some questionable workarounds.
// Not adding deps for now, as it would require either a larger refactor or some questionable workarounds.
// See https://github.com/WordPress/gutenberg/pull/41612 for context.
}, [ activeMenu ] );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const useNavigationTreeItem = (
return () => {
removeItem( itemId );
};
// Ignore exhaustive-deps rule for now. See https://github.com/WordPress/gutenberg/pull/41639
// Not adding deps for now, as it would require either a larger refactor.
// See https://github.com/WordPress/gutenberg/pull/41639
}, [ activeMenu, search ] );
};
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ function MenuTitleSearch( {
count
);
debouncedSpeak( resultsFoundMessage );
// Ignore exhaustive-deps rule for now. See https://github.com/WordPress/gutenberg/pull/44090
// Not adding deps for now, as it would require either a larger refactor.
// See https://github.com/WordPress/gutenberg/pull/44090
}, [ items, search ] );

const onClose = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export const useNavigationTreeMenu = ( props: NavigationMenuProps ) => {
return () => {
removeMenu( key );
};
// Ignore exhaustive-deps rule for now. See https://github.com/WordPress/gutenberg/pull/44090
// Not adding deps for now, as it would require either a larger refactor
// See https://github.com/WordPress/gutenberg/pull/44090
}, [] );
};
1 change: 1 addition & 0 deletions packages/components/src/sandbox/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ const Sandbox = forwardRef( function Sandbox(

useEffect( () => {
updateContentHtml();
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ html, title, type, styles, scripts ] );

useEffect( () => {
Expand Down
6 changes: 6 additions & 0 deletions packages/components/src/sandbox/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -262,14 +262,20 @@ function SandBox( {
checkMessageForResize
);
};
// Passing `exhaustive-deps` will likely involve a more detailed refactor.
// See https://github.com/WordPress/gutenberg/pull/44378
}, [] );

useEffect( () => {
trySandBox();
// Passing `exhaustive-deps` will likely involve a more detailed refactor.
// See https://github.com/WordPress/gutenberg/pull/44378
}, [ title, styles, scripts ] );

useEffect( () => {
trySandBox( true );
// Passing `exhaustive-deps` will likely involve a more detailed refactor.
// See https://github.com/WordPress/gutenberg/pull/44378
}, [ html, type ] );

return (
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/search-control/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ function SearchControl( {
mergeFutureStyles( activeDarkStyles, [ isActive, isDark ] );

setCurrentStyles( futureStyles );
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ isActive, isDark ] );

const clearInput = useCallback( () => {
Expand Down
5 changes: 5 additions & 0 deletions packages/components/src/tooltip/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ const useKeyboardVisibility = () => {
showListener.remove();
hideListener.remove();
};
// See https://github.com/WordPress/gutenberg/pull/41166
}, [] );

return keyboardVisible;
Expand Down Expand Up @@ -102,6 +103,7 @@ const Tooltip = ( {
} );
}
return () => onHandleScreenTouch( null );
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ visible ] );

// Manage visibility animation.
Expand All @@ -115,6 +117,7 @@ const Tooltip = ( {
setAnimating( true );
startAnimation();
}
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ visible ] );

// Manage tooltip visibility and position in relation to keyboard.
Expand All @@ -133,6 +136,7 @@ const Tooltip = ( {
setAnimating( true );
setVisible( false );
}
// See https://github.com/WordPress/gutenberg/pull/41166
}, [ visible, keyboardVisible ] );

// Manage tooltip position during keyboard frame changes.
Expand Down Expand Up @@ -261,6 +265,7 @@ const TooltipSlot = ( { children, ...rest } ) => {
setHandleScreenTouch( null );
};
// Memoize context value to avoid unnecessary rerenders of the Provider's children
// See https://github.com/WordPress/gutenberg/pull/41166
const value = useMemo( () => ( { onHandleScreenTouch } ) );

return (
Expand Down
4 changes: 4 additions & 0 deletions packages/components/src/unit-control/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ function UnitControl( {
if ( pickerRef?.current ) {
pickerRef.current.presentPicker();
}
// It would be great if the deps could be addressed in the context of
// https://github.com/WordPress/gutenberg/pull/39218
}, [ pickerRef?.current ] );

const currentInputValue = currentInput === null ? value : currentInput;
Expand Down Expand Up @@ -102,6 +104,8 @@ function UnitControl( {
anchorNodeRef?.current
? findNodeHandle( anchorNodeRef?.current )
: undefined,
// It would be great if the deps could be addressed in the context of
// https://github.com/WordPress/gutenberg/pull/39218
[ anchorNodeRef?.current ]
);

Expand Down

0 comments on commit 544528b

Please sign in to comment.