Skip to content

Update paddings and theme handling for rich text editor toolbar#4852

Merged
vhuseinova-msft merged 18 commits intomainfrom
vhuseinova/rich-text-editor-padding-fix
Jul 15, 2024
Merged

Update paddings and theme handling for rich text editor toolbar#4852
vhuseinova-msft merged 18 commits intomainfrom
vhuseinova/rich-text-editor-padding-fix

Conversation

@vhuseinova-msft
Copy link
Copy Markdown
Member

What

  • fix for rtl and ltr modes
  • updated paddings/margins for rich text editor toolbar

Why

How Tested

storybook

Process & policy checklist

  • I have updated the project documentation to reflect my changes if necessary.
  • I have read the CONTRIBUTING documentation.

Is this a breaking change?

  • This change causes current functionality to break.

@vhuseinova-msft vhuseinova-msft requested review from a team as code owners July 12, 2024 16:47
@vhuseinova-msft vhuseinova-msft added the update_snapshots Set this label to request automated update of UI snapshots label Jul 12, 2024
@github-actions github-actions Bot removed the update_snapshots Set this label to request automated update of UI snapshots label Jul 12, 2024
@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 12, 2024

Chat bundle size is increased❗.

  • Current size: 2109505
  • Base size: 2108366
  • Diff size: 1139

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 12, 2024

CallWithChat bundle size is increased❗.

  • Current size: 6443411
  • Base size: 6442272
  • Diff size: 1139

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 12, 2024

Calling bundle size is not changed.

  • Current size: 5086669
  • Base size: 5086669
  • Diff size: 0

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 12, 2024

@azure/communication-react jest test coverage for stable.

Lines Statements Functions Branches
Base 26598 / 42080
63.2%
26598 / 42080
63.2%
720 / 1298
55.46%
2121 / 3353
63.25%
Current 26662 / 42097
63.33%
26662 / 42097
63.33%
721 / 1299
55.5%
2130 / 3367
63.26%
Diff 64 / 17
0.13%
64 / 17
0.13%
1 / 1
0.04%
9 / 14
0.01%

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 12, 2024

@azure/communication-react jest test coverage for beta.

Lines Statements Functions Branches
Base 52505 / 84544
62.1%
52505 / 84544
62.1%
1066 / 2367
45.03%
3128 / 5123
61.05%
Current 52489 / 84560
62.07%
52489 / 84560
62.07%
1067 / 2368
45.05%
2997 / 5055
59.28%
Diff -16 / 16
-0.03%
-16 / 16
-0.03%
1 / 1
0.02%
-131 / -68
-1.77%

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

// check that editor exists and theme was actually changed
if (editor.current && previousThemeDirection.current !== themeDirectionValue) {
const format = getFormatState(editor.current);
if (format.direction !== themeDirectionValue) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't checking format.direction already sufficient? Do we need to introduce previousThemeDirection?

Copy link
Copy Markdown
Member Author

@vhuseinova-msft vhuseinova-msft Jul 15, 2024

Choose a reason for hiding this comment

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

Unfortunately, it's not sufficient as format.direction is set only after set direction function is called and that's why we need to upderstand if this a real change of theme direction or it's just an initial theme value for this component

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

One of the issues about calling setDirection when setting the initial value of theme is that setDirection uses focus call inside and it won't work very well with our autoFocus prop

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unfortunately, it's not sufficient as format.direction is set only after set direction function is called and that's why we need to upderstand if this a real change of theme direction or it's just an initial theme value for this component

Should we add a comment here to document this information? I feel it's quite useful to remember this finding.

@vhuseinova-msft vhuseinova-msft enabled auto-merge (squash) July 15, 2024 23:21
@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@vhuseinova-msft vhuseinova-msft merged commit 62477ca into main Jul 15, 2024
@vhuseinova-msft vhuseinova-msft deleted the vhuseinova/rich-text-editor-padding-fix branch July 15, 2024 23:34
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 this pull request may close these issues.

3 participants