Skip to content
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

Jump to bottom

[RNMobile] Fix issue related to receiving undefined ref in text color format #56686

Merged
merged 5 commits into from
Dec 1, 2023
Merged

[RNMobile] Fix issue related to receiving undefined ref in text color format #56686

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
91e9e27
Fix issue related to receiving undefined ref in text color format
fluiddot Nov 30, 2023
ac58714
Update `react-native-editor` changelog
fluiddot Nov 30, 2023
a2b4784
Add test to cover undefined `contentRef` case
fluiddot Nov 30, 2023
c79cca2
Correct typo in `changelog`
fluiddot Nov 30, 2023
72b276a
Merge branch 'trunk' into rnmobile/fix/text-color-format-props-undefined
fluiddot Dec 1, 2023
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
4 changes: 1 addition & 3 deletions packages/format-library/src/text-color/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ const name = 'core/text-color';
const title = __( 'Text color' );

function getComputedStyleProperty( element, property ) {
const {
props: { style = {} },
} = element;
const style = element?.props?.style ?? {};
Copy link
Contributor Author

@fluiddot fluiddot Nov 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This logic relies on the fact that the ref object (contentRef), passed from the RichText component, contains the props passed to the AztecView component.

gutenberg/packages/format-library/src/text-color/index.native.js

Lines 51 to 58 in a2b4784

function fillComputedColors( element, { color, backgroundColor } ) {
if ( ! color && ! backgroundColor ) {
return;
}
return {
color: color || getComputedStyleProperty( element, 'color' ),
backgroundColor: getComputedStyleProperty(

gutenberg/packages/format-library/src/text-color/index.native.js

Lines 65 to 86 in a2b4784

function TextColorEdit( {
value,
onChange,
isActive,
activeAttributes,
contentRef,
} ) {
const [ allowCustomControl ] = useSettings( 'color.custom' );
const colors = useMobileGlobalStylesColors();
const [ isAddingColor, setIsAddingColor ] = useState( false );
const enableIsAddingColor = useCallback(
() => setIsAddingColor( true ),
[ setIsAddingColor ]
);
const disableIsAddingColor = useCallback(
() => setIsAddingColor( false ),
[ setIsAddingColor ]
);
const colorIndicatorStyle = useMemo(
() =>
fillComputedColors(
contentRef,

gutenberg/packages/block-editor/src/components/rich-text/native/format-edit.js

Line 40 in a2b4784

contentRef={ forwardedRef }

gutenberg/packages/block-editor/src/components/rich-text/native/index.native.js

Lines 1336 to 1337 in a2b4784

<FormatEdit
forwardedRef={ this._editor }

gutenberg/packages/block-editor/src/components/rich-text/native/index.native.js

Lines 1269 to 1272 in a2b4784

<RCTAztecView
accessibilityLabel={ accessibilityLabel }
ref={ ( ref ) => {
this._editor = ref;

In theory, the props are not exposed in ref objects, so I investigated further where are they coming from. I couldn't pinpoint the code from React/React Native but I confirmed that ref objects coming from native components (like Aztec or Video (react-native-video) contain extra properties like props or forceUpdate. On the other hand, I couldn't find any documentation related to this, so I assume this approach is not recommended and might lead to issues in future React Native versions.


if ( property === 'background-color' ) {
const { backgroundColor, baseColors } = style;
Expand Down
64 changes: 62 additions & 2 deletions packages/format-library/src/text-color/test/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,29 @@
* External dependencies
*/
import {
fireEvent,
getEditorHtml,
initializeEditor,
fireEvent,
render,
waitFor,
} from 'test/helpers';

/**
* WordPress dependencies
*/
import { setDefaultBlockName, unregisterBlockType } from '@wordpress/blocks';
import {
registerBlockType,
setDefaultBlockName,
unregisterBlockType,
} from '@wordpress/blocks';
import { coreBlocks } from '@wordpress/block-library';
import { BlockControls, BlockEdit } from '@wordpress/block-editor';
import { SlotFillProvider } from '@wordpress/components';

/**
* Internal dependencies
*/
import { textColor } from '..';

const COLOR_PINK = '#f78da7';
const paragraph = coreBlocks[ 'core/paragraph' ];
Expand Down Expand Up @@ -164,4 +176,52 @@ describe( 'Text color', () => {

expect( getEditorHtml() ).toMatchSnapshot();
} );

it( 'renders when "contentRef" is undefined', () => {
registerBlockType( 'core/test-block', {
save: () => {},
category: 'text',
title: 'block title',
edit: ( { children } ) => <>{ children }</>,
} );
const TextColorEdit = textColor.edit;
// Empty text with black color set as the text color
const textValue = {
formats: [],
replacements: [],
text: '',
start: 0,
end: 0,
activeFormats: [
{
type: 'core/text-color',
attributes: {
style: 'background-color:rgba(0, 0, 0, 0);color:#111111',
class: 'has-contrast-color',
},
},
],
};

const { getByLabelText } = render(
<SlotFillProvider>
<BlockEdit name="core/test-block" isSelected>
<TextColorEdit
isActive={ true }
activeAttributes={ {} }
value={ textValue }
onChange={ jest.fn() }
// This ref is usually defined by the `RichText` component.
// However, there are rare cases (probably related to slow performance
// in low-end devices) where it's undefined upon mounting.
contentRef={ undefined }
/>
</BlockEdit>
<BlockControls.Slot />
</SlotFillProvider>
);

const textColorButton = getByLabelText( 'Text color' );
expect( textColorButton ).toBeDefined();
} );
} );
1 change: 1 addition & 0 deletions packages/react-native-editor/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ For each user feature we should also add a importance categorization label to i

## Unreleased
- [***] Fix issue when backspacing in an empty Paragraph block [#56496]
- [**] Fix issue related to text color format and recieving in rare cases an undefined ref from `RichText` component [#56686]

## 1.109.0
- [*] Audio block: Improve legibility of audio file details on various background colors [#55627]
Expand Down
Loading