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

Custom Branding bug bash fixes - Visual improvements to the call composite configuration page #3875

Merged
merged 21 commits into from
Dec 11, 2023

Conversation

JamesBurnside
Copy link
Member

@JamesBurnside JamesBurnside commented Dec 7, 2023

What

  • Add text shadow to config page call description (the secondary text)
  • Fix visual artifact on the configuration page section rounded borders
    • the inner border was overlapping the outer border, updated to use overflow:hidden to ensure the child background does not overflow the rounded border
    • Updated to use theme.effects.roundedCorner6 to ensure rounded corners follow theme
  • Fix the localpreview styles to center align the text
  • Fix title not rendering on safari -- safari doesn't support rem as a dy value in svg, using px instead

Why

Fixes design bugs.

How Tested

Locally verified new alignments match designs:

(left design, right actual)
image

@JamesBurnside JamesBurnside added the update_snapshots Set this label to request automated update of UI snapshots label Dec 7, 2023
@github-actions github-actions bot removed the update_snapshots Set this label to request automated update of UI snapshots label Dec 7, 2023
Copy link
Contributor

github-actions bot commented Dec 7, 2023

Copy link
Contributor

github-actions bot commented Dec 7, 2023

Calling bundle size is increased❗.

  • Current size: 5587801
  • Base size: 5587766
  • Diff size: 35

Copy link
Contributor

github-actions bot commented Dec 7, 2023

CallWithChat bundle size is increased❗.

  • Current size: 6400985
  • Base size: 6400950
  • Diff size: 35

Copy link
Contributor

github-actions bot commented Dec 7, 2023

Chat bundle size is not changed.

  • Current size: 1526469
  • Base size: 1526469
  • Diff size: 0

Copy link
Contributor

github-actions bot commented Dec 7, 2023

Copy link
Contributor

github-actions bot commented Dec 7, 2023

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

Lines Statements Functions Branches
Base 21200 / 33186
63.88%
21200 / 33186
63.88%
579 / 1003
57.72%
1683 / 2684
62.7%
Current 21224 / 33186
63.95%
21224 / 33186
63.95%
579 / 1003
57.72%
1692 / 2684
63.04%
Diff 24 / 0
0.07%
24 / 0
0.07%
0 / 0
0%
9 / 0
0.34%

Copy link
Contributor

github-actions bot commented Dec 7, 2023

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

Lines Statements Functions Branches
Base 43610 / 68809
63.37%
43610 / 68809
63.37%
906 / 1947
46.53%
2602 / 4189
62.11%
Current 43564 / 68816
63.3%
43564 / 68816
63.3%
906 / 1952
46.41%
2587 / 4177
61.93%
Diff -46 / 7
-0.07%
-46 / 7
-0.07%
0 / 5
-0.12%
-15 / -12
-0.18%

@JamesBurnside JamesBurnside marked this pull request as ready for review December 7, 2023 20:34
@JamesBurnside JamesBurnside requested review from a team as code owners December 7, 2023 20:34
Copy link
Contributor

github-actions bot commented Dec 7, 2023

@@ -109,7 +109,7 @@ const constructTSpanLine = (line: string, lineHeightPx: number): SVGTSpanElement
const tspan = document.createElementNS('http://www.w3.org/2000/svg', 'tspan');
tspan.textContent = line;
tspan.setAttribute('x', '50%');
tspan.setAttribute('dy', _pxToRem(lineHeightPx));
tspan.setAttribute('dy', `${lineHeightPx}px`);
Copy link
Member

Choose a reason for hiding this comment

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

is this change happening because the SVG format does not like REM (Great band)

Copy link
Member Author

Choose a reason for hiding this comment

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

Specifically safari doesn't support rem as a dy value in svg

Copy link
Member Author

Choose a reason for hiding this comment

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

(bug in webkit)

@JamesBurnside JamesBurnside added the update_snapshots Set this label to request automated update of UI snapshots label Dec 11, 2023
@github-actions github-actions bot removed the update_snapshots Set this label to request automated update of UI snapshots label Dec 11, 2023
Copy link
Contributor

Copy link
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.

Copy link
Contributor

Copy link
Contributor

@JamesBurnside JamesBurnside enabled auto-merge (squash) December 11, 2023 17:55
@JamesBurnside JamesBurnside merged commit 257b695 into main Dec 11, 2023
36 checks passed
@JamesBurnside JamesBurnside deleted the jaburnsi/branding-bug-bash-fixes branch December 11, 2023 18:15
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