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.

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

Bug: $getSelectionStyleValueForProperty(selection, 'font-size') not updating correctly #4491

Closed
jonesguy14 opened this issue May 12, 2023 · 15 comments · Fixed by #5082
Closed
Assignees

Comments

@jonesguy14
Copy link

jonesguy14 commented May 12, 2023

When using $getSelectionStyleValueForProperty(selection, 'font-size'), it does not seem to update if user changes font size on some text which has previously modified its font size. The font-family property also exhibits the same behavior.

Lexical version: 0.10.0

Steps To Reproduce

This can be reproduced in the https://playground.lexical.dev/:

  1. Open lexical playground
  2. Change font size for some text
  3. Click away from the text
  4. Highlight that same text again, and change the font size

I've also created a sandbox, see FontSizePlugin: https://codesandbox.io/s/dazzling-visvesvaraya-8pjcv5

Repro in sandbox by:

  1. Type some text and highlight it
  2. Change font size to 24px
  3. Deselect text and reselect
  4. Change font size to 48px

See that the text font size does change, but the getSelectionStyleValueForProperty result still says '24px'.

The current behavior

Toolbar does not update, but text font size does change. getSelectionStyleValueForProperty seems to still return the original value.

The expected behavior

getSelectionStyleValueForProperty will return the new font size, so that the toolbar can update to new font size.

This only happens for text that has font size changed, the initial change from no font-size to some value does seem to update correctly.

@GAURAV1-ui
Copy link

Can I work on this issue.

@acywatson
Copy link
Contributor

Can I work on this issue.

Sure - assigned to you

@jonesguy14
Copy link
Author

This also reproduces for the font-family property, updated in description. Hoping the fix will be for all $getSelectionStyleValueForProperty property types.

@greeneley
Copy link

This issue also happens with me. Select a text -> change the font-size -> de-select text -> re-select this text -> change the font-size -> I see the toolbar doens't update the according font-size although the text font size in editor does change.
My using lexical version is 0.11.1

@greeneley
Copy link

Hi @GAURAV1-ui,
How about the pull request ?

@MaximMukhanov
Copy link

MaximMukhanov commented Jul 20, 2023

@GAURAV1-ui
It seems to be a problem with style property on RangeSelection, it seems to not update on style change.
Hope it helps!

@XFG16
Copy link

XFG16 commented Aug 3, 2023

Is there any fix to this as of now?

@jonesguy14
Copy link
Author

@GAURAV1-ui any progress here?

@acywatson can this get unassigned/reassigned if not?

@jonesguy14
Copy link
Author

This is what I did for a workaround. Seems like the RangeSelection style isn't being updated when $patchStyleText is called, so I just call setStyle on it manually.

// Use this function instead of $patchStyleText
export default function $customPatchStyleText(
	selection: RangeSelection,
	cssProperty: string,
	cssValue: string
) {
	$patchStyleText(selection, { [cssProperty]: cssValue });

	const newStyle = replaceCssStyle(selection.style, cssProperty, cssValue);
	selection.setStyle(newStyle);
}

function replaceCssStyle(
	existingCssStyle: string,
	cssProperty: string,
	newCssValue: string
): string {
	const cssArr = existingCssStyle
		.split(';')
		.filter((prop) => !!prop)
		.map((prop) => prop.trim());

	let found = false;

	const newCssArr = cssArr.map((prop) => {
		const propertySplit = prop.split(':');
		if (propertySplit.length !== 2) {
			return '';
		}

		const [property, value] = propertySplit.map((p) => p.trim());

		if (property === cssProperty) {
			found = true;
			return `${property}: ${newCssValue}`;
		} else {
			return `${property}: ${value}`;
		}
	});

	// If the property was not found in the existing styles, add it
	if (!found) {
		newCssArr.push(`${cssProperty}: ${newCssValue}`);
	}

	return newCssArr.filter((value) => value !== '').join('; ');
}

replaceCssStyle could be done with regex too 🤷.

This seems to work well, as I have another hook which listens to editor updates and calls $getSelectionStyleValueForProperty for various CSS properties, and now that function call resolves as expected with the new styles.

There may be / probably is a better solution in the core framework layer, but as a workaround it does the job.

@Piliuta
Copy link
Contributor

Piliuta commented Sep 27, 2023

This seems to be a larger problem with the selection that does not reflect the correct inline style value. I have other steps to reproduce the issue. Testes on the playground https://playground.lexical.dev/

Screen.Recording.2023-09-27.at.13.44.57.mov

@acywatson It's been 4+ months since it was assigned to @GAURAV1-ui but no updates. Is it possible for somebody from the lexical core team to work on it?

@roeycohen
Copy link

hi @Piliuta and @acywatson,

is this merge already in version 0.12.2?
because it seems like $patchStyleText still doesn't work well with font-family, and using selection.setStyle like in @jonesguy14 solution works well...

Thanks!

@Piliuta
Copy link
Contributor

Piliuta commented Nov 16, 2023

hi @Piliuta and @acywatson,

is this merge already in version 0.12.2? because it seems like $patchStyleText still doesn't work well with font-family, and using selection.setStyle like in @jonesguy14 solution works well...

Thanks!

The fix was merged after the 0.12.2 release and there are no new releases, so it is not fixed in the latest release yet.

@acywatson any ETA for the next release?

@roeycohen
Copy link

Hi @Piliuta,

Do you mind explaining why this PR actually fix the bug? :)
What's the difference between background-color and font-family and how it is related to the selection being collapsed or not.

Thanks,
Roey

@Piliuta
Copy link
Contributor

Piliuta commented Nov 20, 2023

Hi @Piliuta,

Do you mind explaining why this PR actually fix the bug? :) What's the difference between background-color and font-family and how it is related to the selection being collapsed or not.

Thanks, Roey

They are both "style" properties, so they are the same from the selection perspective. There is a bug in the core functionality that does not update the property value when the selection is not collapsed. The PR fixes the $getSelectionStyleValueForProperty method so it does not return value from the style property of the selection but calculates the value from the selection state.

@roeycohen
Copy link

thanks @Piliuta for taking your time and answering me :)
I've installed the latest version from 3 days ago (0.12.4) and it works perfectly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants