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

Improve Mark formatting #1073

Merged
merged 13 commits into from Mar 21, 2024
Merged

Improve Mark formatting #1073

merged 13 commits into from Mar 21, 2024

Conversation

geriux
Copy link
Member

@geriux geriux commented Jan 19, 2024

Related PRs:

This PR adds missing implementation for the Mark formatting style.

  • Adds a new MarkPlugin that will handle adding the needed CSS attributes.
  • Updates the MarkSpan to receive a color param and to set it as the current color.
  • Removes applyMarkInlineStyle in favor of applyAfterMarkInlineStyle which will handle if the color changes while editing a Mark span.

Test

Please follow the testing instructions in this PR's description.

This shouldn't affect other Aztec functionalities.

Make sure strings will be translated:

  • If there are new strings that have to be translated, I have added them to the client's strings.xml as a part of the integration PR.

@geriux
Copy link
Member Author

geriux commented Mar 18, 2024

Hey there @antonis 👋

I was wondering if you would be able to help me out with a PR review 😄, I've updated the Highlight color implementation so it fixes some issues for this feature since we first partially implemented it in #943

A lot of testing has been already done in WordPress/gutenberg#57650 so I was wondering if you'd have some time to check the native code for the Aztec side. Let me know if you could take a look! Thank you!

@antonis antonis self-requested a review March 19, 2024 06:09
@antonis
Copy link
Member

antonis commented Mar 19, 2024

I was wondering if you'd have some time to check the native code for the Aztec side. Let me know if you could take a look! Thank you!

I'll be glad to help with this @geriux :) I plan to get to this later today or tomorrow 🙇

@geriux
Copy link
Member Author

geriux commented Mar 19, 2024

I'll be glad to help with this @geriux :) I plan to get to this later today or tomorrow 🙇

Thank you so much!

Copy link
Member

@antonis antonis left a comment

Choose a reason for hiding this comment

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

Awesome work @geriux
The code overall looks good and works as expected. I added some suggestions which are not blocking.

@geriux
Copy link
Member Author

geriux commented Mar 21, 2024

Thank you for all of the suggestions @antonis! I've applied them in 17946f2 and 2f73564 🚀

@geriux geriux merged commit 6e9814f into trunk Mar 21, 2024
14 checks passed
@geriux geriux deleted the fix/mark-style-issues branch March 21, 2024 11:32
@antonis antonis mentioned this pull request Apr 23, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants