Persona: complete mergeStyles conversion and some small refactoring, add some styling functions. #4382
Conversation
from IPersonaSharedProps to IPersonaProps
| }; | ||
|
|
||
| // Use getStyles from props, or fall back to getStyles from styles file. | ||
| const classNames = getClassNames(getStylesProp || getStyles, { |
There was a problem hiding this comment.
@dzearing In this case, I am using the same file for PersonaCoinBase and PersonaCoin, so getStyles is directly imported from the PersonaCoin.styles.ts file. Do I need to do this (getStylesProp || getStyles) or does the styled function already take care of getStyles from props?
There was a problem hiding this comment.
The styled function already combines getStyles from props! :)
- Remove redundant code - Switch to system fonts only - Switch High Contrast colors to agreed new color scheme. - Available presence is now Highlight - All other presence are WindowText - Use semanticColors instead of palette colors - Remove HighContrastBWSelector - Blocked status match Fabric toolkit
|
@phkuo I removed the custom contrast colors and went to just system colors. @betrue-final-final and I conversed offline and decided that the presence colors should be I also have some work stashed to help with accessibility and determining presence status (especially in high contrast mode where the colors are now system colors only). Basically, I have the presence component have a |
| * background, whether a <color> or an <image>, extends | ||
| * underneath its border. | ||
| * | ||
| * The `text` value is experimental and should not be used |
There was a problem hiding this comment.
should also note it doesn't work in IE
| }; | ||
|
|
||
| export const noWrap: IRawStyle = { | ||
| display: 'block', |
There was a problem hiding this comment.
probably would prefer inline-block here, so things don't take up all available width
although i'd prefer just leaving off display altogether and letting the consumer decide between block/inline-block
There was a problem hiding this comment.
Probably a good idea. Thanks for all your thorough reviewing!
7 participants
Pull request checklist
$ npm run changeDescription of changes
Convert Persona, PersonaCoin, and PersonaPresence to mergeStyles:
IPersonaSharedPropsthat is extended byIPersonaProps,IPersonaCoinProps,IPersonaPresenceProps.IRenderFunction<IPersonaProps>and are used in Persona AND PersonaCoin, such asonRenderCoin, were changed toIRenderFunction<IPersonaSharedProps>.DeletePersonaConsts.ts. It was only applying className logic.PersonaConsts.tsto serve size and presence states to styles files.rootIsDarkText,rootIsSelectable,.rootIsReadonlywere in the sass files, but never called from the component, and not accepted in the types.PersonaSizeenum orcoinSize <= 32).Added functionality:
noWrapstyles in Styling (Mimics noWrap sass mixin)zIndexnamespace in Styling (Mimics$ms-zIndexsass variables)Add High Contrast colors toDefaultPalette(were missing from sass variable conversion)Other:
IPersonaSharedPropsdive()through decorated Personas.Focus areas to test