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

fix(NcRich*): style fixes and RTL support #6474

Merged
merged 5 commits into from
Jan 31, 2025
Merged

Conversation

Antreesy
Copy link
Contributor

☑️ Resolves

  • drop unused prop markdownCssClasses and styles for it
  • we did not support custom classes since component was merged, so should not be breaking
  • apply logical rules to Rich components
  • decrease size of 'tab' symbol (from 8 to 4 spaces)
  • increase padding for list items (from 15px to 4ch)
    • closer to native ~40px from user agent

🖼️ Screenshots

🏚️ Before 🏡 After
image image
image image
image image

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 3️⃣ Backport to next requested with a Vue 3 upgrade

- we did not support custom classes since component was merged, so should not be breaking

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
- closer to native ~40px from user agent

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy Antreesy added bug Something isn't working 3. to review Waiting for reviews feature: rich-contenteditable Related to the rich-contenteditable components feature: richtext Related to the richtext component RTL Right-to-Left languages support labels Jan 29, 2025
@Antreesy Antreesy added this to the 8.23.0 milestone Jan 29, 2025
@Antreesy Antreesy self-assigned this Jan 29, 2025
@@ -1117,7 +1118,8 @@ export default {
overflow: auto;
// Hide container root element while initializing
position: absolute;
left: -10000px;
/* stylelint-disable csstools/use-logical */ /* upstream logic */
left: -100vw;
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this needed? Cant you just use inset-inline-start?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

.tribute-container applies computed styles to position itself with left.
With RTL it's going to be left: <calculated>px; right: -100vw;, so it will be out of screen

Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

Looks good to me

Copy link
Contributor

@DorraJaouad DorraJaouad left a comment

Choose a reason for hiding this comment

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

Tested with Arabic, no issues.

@Antreesy Antreesy merged commit ab614bf into master Jan 31, 2025
23 checks passed
@Antreesy Antreesy deleted the fix/noid/rich-bidi-support branch January 31, 2025 15:04
@Antreesy
Copy link
Contributor Author

/backport to next

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working feature: rich-contenteditable Related to the rich-contenteditable components feature: richtext Related to the richtext component RTL Right-to-Left languages support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants