fix(Chat): update ChatMessage styles in Teams themes#1246
fix(Chat): update ChatMessage styles in Teams themes#1246layershifter merged 10 commits intomasterfrom
Conversation
layershifter
requested review from
dvdzkwsk,
kuzhelov,
levithomason,
miroslavstastny and
mnajdova
as code owners
layershifter
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1246 +/- ##
======================================
Coverage 72.1% 72.1%
======================================
Files 731 731
Lines 5592 5592
Branches 1612 1634 +22
======================================
Hits 4032 4032
Misses 1555 1555
Partials 5 5
Continue to review full report at Codecov.
|
| authorMarginRight: pxToRem(12), | ||
| authorFontWeight: siteVars.fontWeightBold, | ||
| authorColor: siteVars.gray02, | ||
| authorFontWeight: siteVars.fontWeightRegular, |
There was a problem hiding this comment.
Is this even needed if it's regular?
There was a problem hiding this comment.
You're right, we don't need it. But we already have this variable and I don't want to remove it because it will be a breaking change...
| authorColor: siteVars.gray02, | ||
| authorFontWeight: siteVars.fontWeightRegular, | ||
| headerMarginBottom: pxToRem(2), | ||
| contentColor: siteVars.colors.grey[900], |
There was a problem hiding this comment.
This hex value is actually slightly wrong, but it should be fixed with the new color palette updates (#252423 instead of #252424. Can you add a comment here so we know that when the color palette updates go in we should be using gray[750]?
There was a problem hiding this comment.
Replaced with direct value and added a note 👍
| return { | ||
| backgroundColor: siteVars.gray10, | ||
| backgroundColorMine: '#3B3C54', | ||
| authorColor: siteVars.gray02, |
There was a problem hiding this comment.
Not sure if it is helpful to @mnajdova , but this will be gray[250] with new color palette
| padding: pxToRem(16), | ||
| authorMarginRight: pxToRem(12), | ||
| authorFontWeight: siteVars.fontWeightBold, | ||
| authorColor: siteVars.gray02, |
There was a problem hiding this comment.
Not sure if it is helpful to @mnajdova , but will be gray[500] with new color palette
There was a problem hiding this comment.
I added comments for these values
| authorColor: siteVars.gray02, // will be gray[500] with new palette | ||
| authorFontWeight: siteVars.fontWeightRegular, | ||
| headerMarginBottom: pxToRem(2), | ||
| contentColor: '#252423', // will be gray[750] with new palette |
There was a problem hiding this comment.
There is a color variable here too. I wonder if we don't need this variable if the color prop above is set to this value.
There was a problem hiding this comment.
But color is rgb(64, 64, 64) that points to #404040...
| onBlur: PropTypes.func, | ||
| onFocus: PropTypes.func, | ||
| reactionGroup: customPropTypes.itemShorthand, | ||
| reactionGroup: PropTypes.oneOfType([ |
There was a problem hiding this comment.
👍 My bad for not updating this change. We should update this on other places (Menu is the first one that pops out of my mind), but I will take care of those changes.
…tardust-ui/react into fix/chat-styles # Conflicts: # CHANGELOG.md
3 participants
This PR:
ChatMessagein Teams, Teams Dark and Teams HC themesTeams theme
Teams Dark theme
Teams HC theme