fix(Chat): update ChatMessage styles in Teams themes #1246
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.
|
| @@ -37,8 +39,10 @@ export default (siteVars): ChatMessageVariables => ({ | |||
| offset: pxToRem(100), | |||
| padding: pxToRem(16), | |||
| 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...
| 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 👍
| @@ -4,6 +4,8 @@ export default (siteVars: any): Partial<ChatMessageVariables> => { | |||
| 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
| @@ -37,8 +39,10 @@ export default (siteVars): ChatMessageVariables => ({ | |||
| offset: pxToRem(100), | |||
| 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
| 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...
| @@ -121,7 +121,10 @@ class ChatMessage extends UIComponent<ReactProps<ChatMessageProps>, ChatMessageS | |||
| timestamp: customPropTypes.itemShorthand, | |||
| 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
…tardust-ui/react into fix/chat-styles
This PR:
ChatMessagein Teams, Teams Dark and Teams HC themesTeams theme
Teams Dark theme
Teams HC theme