RFC: Reusing Avatar in other components#24790
Conversation
This RFC presents the problem of Avatar reuse when trying to follow design guidelines. Several solutions are proposed along with their pros/cons
📊 Bundle size report🤖 This report was generated against 48df172dd62d2bc71409c8cdb3967ea500dcd999 |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 2feea25:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: c37c8714056e1e021ac7da86ca4bec3428e1953a (build) |
|
|
||
| - Extra code in base component | ||
| - Needs prototyping and investigation | ||
| - All avatars under this context will be affected |
There was a problem hiding this comment.
I suggest to add example with Popover there - an Avatar inside a Popover will have size "20" instead of default one
There was a problem hiding this comment.
As I implemented in the prototype, the choice of slot to apply this context can be more 'intelligent' #24807, and honestly it might not be that bad 🤔
Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
…fluentui into rfc/avatar-icon-sizing
|
I believe the Shared context would be the best solution described here so far. Pros
Cons
|
| - Cannot ship design updates i.e. users will need to upgrade all usages if design decides that "64px" will be new size | ||
| - Extra work for users | ||
|
|
||
| ### Style override |
There was a problem hiding this comment.
Have we considered the usage of CSS variables for the style overrides proposal? This solves the "Does not handle props" con. If the size prop of the avatar adds styles with the CSS variable, it'll have priority over the overriden style by the Table.
A con to this approach is to document well the CSS variable.
There was a problem hiding this comment.
The problem with style overrides (I tried it this morning) is that for Avatar the size actually controls multiple css properties:
- width
- height
- icon slot size
- font size
- font weight
I don't think a single css var will actually help and it's a problem that @behowell highlighted offline during tech sync. Furthermore, if designers decide to update the styles with size, it's harder to know what properties to have to update
|
I am for the React context option 👍 |
* RFC: Reusing Avatar in other components This RFC presents the problem of Avatar reuse when trying to follow design guidelines. Several solutions are proposed along with their pros/cons * update * update * Update rfcs/react-components/convergence/reusing-avatar-in-components.md Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com> * add prototype implemenentation of context solution * update code sample * update preferred solution Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
This RFC presents the problem of Avatar reuse when trying to follow design guidelines. Several solutions are proposed along with their pros/cons
PREVIEW