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

feat: use fluentui to theme the person component. #2072

Merged
merged 44 commits into from
Apr 25, 2023

Conversation

musale
Copy link
Contributor

@musale musale commented Mar 2, 2023

Closes #2073, Closes #2117

PR Type

  • Feature

Description of the changes

  • Updated the person component to use fluent UI colors for theming.
  • Renamed current mgt tokens to use the person-* format for person component.
  • Pruned the tokens to only the necessary ones.

PR checklist

  • Project builds (yarn build) and changes have been tested in at least two supported browsers (Edge + non-Chromium based browser)
  • All public APIs (classes, methods, etc) have been documented following the jsdoc syntax
  • Stories have been added and existing stories have been tested
  • Added appropriate documentation. Docs PR:
  • License header has been added to all new source files (yarn setLicense)
  • Contains NO breaking changes

Other information

I'm yet to fix usages of the person in other components. Still a WIP.

  • mgt-people
  • mgt-agenda
  • mgt-login
  • mgt-people-picker
  • mgt-person-card

@ghost
Copy link

ghost commented Mar 2, 2023

Thank you for creating a Pull Request @musale.

This is a checklist for the PR reviewer(s) to complete before approving and merging this PR:

  • I have verified a documentation PR has been linked and is approved (or not applicable)
  • I have ran this PR locally and have tested the fix/feature
  • I have verified that stories have been added to storybook (or not applicable)
  • I have tested existing stories in storybook to verify no regression has occured
  • I have tested the solution in at least two browsers (Edge + 1 non-Chromium based browser)

gavinbarron and others added 2 commits March 2, 2023 18:25
* ci setup pipeline for preview release

* altering setVersion script to ensure four part numeric version in package-solution.json

* adding picker to the generated React code

* fixing setversion logic and adding theme toggle
@sebastienlevert
Copy link
Contributor

😃 waiting the the Storybook 😃

@github-actions
Copy link

github-actions bot commented Mar 2, 2023

The updated storybook is available here

1 similar comment
@github-actions
Copy link

github-actions bot commented Mar 2, 2023

The updated storybook is available here

@github-actions
Copy link

github-actions bot commented Mar 2, 2023

The updated storybook is available here

@sebastienlevert
Copy link
Contributor

I feel we should never set a background color to the control. It should use the app's background color, right? Though, overall we are absolutely on the right track!

@musale
Copy link
Contributor Author

musale commented Mar 2, 2023

I feel we should never set a background color to the control. It should use the app's background color, right? Though, overall we are absolutely on the right track!

I agree with you. For dev purposes, I start it out with a bg therefore before merge, I will ensure it's not set to be the same as the bg.

@github-actions
Copy link

github-actions bot commented Mar 2, 2023

The updated storybook is available here

@github-actions
Copy link

The updated storybook is available here

@github-actions
Copy link

The updated storybook is available here

* fix person-card hover state styling

* update: person card theming changes

* add new person-card customization tokens

* theme chat input, include new css props

* theme the chat input border

* chore: fix location icon color and refactor code

* Update packages/mgt-components/src/utils/SvgHelper.ts

* fix hover styles

* fix perosn-card file-list section color

* fix file-list item z-index issue

* refactor fluent-tabs activeIndicator style padding

* use hidden svgs to fix the hover fill persistence issue

* Fix section nav border z-index issue

* Fix section nav border z-index issue

* update fluent-button aria labels and divider color

* update profile section tokens

* update fluent-button aria-labels

* fix build

* Fix build, remove fullView styles

* fix eslint

* use similar view for files compact and fullView, remove logging

* fix additional eslint errors

---------

Co-authored-by: Martin Musale <martinmusale@microsoft.com>
@github-actions
Copy link

The updated storybook is available here

@musale musale marked this pull request as ready for review April 20, 2023 12:59
@musale musale requested a review from gavinbarron April 20, 2023 12:59
@github-actions
Copy link

The updated storybook is available here

@sebastienlevert
Copy link
Contributor

@musale There seems to have an issue with the presence badge on small avatars story.

image

Is this related to the tokens or to the updates we made to the components?

@@ -6,7 +6,7 @@ name: Deploy pr storybook
on:
pull_request:
types: [opened, labeled, synchronize, reopened]
branches: [main, release/**, next/**]
branches: ['**']
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm?

@@ -7,7 +7,7 @@ name: Build pr

on:
pull_request:
branches: [main, release/**, next/**, feat/add-jest-tests]
branches: ['**']
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was to enable testing the sub-branches from this one. Reverting to main, release and next only.

@musale
Copy link
Contributor Author

musale commented Apr 25, 2023

@musale There seems to have an issue with the presence badge on small avatars story.

image

Is this related to the tokens or to the updates we made to the components?

It's a positioning issue for initials and contact icon avatars. It's now fixed.

@github-actions
Copy link

The updated storybook is available here

Signed-off-by: Musale Martin <martinmusale@microsoft.com>
Signed-off-by: Musale Martin <martinmusale@microsoft.com>
@github-actions
Copy link

The updated storybook is available here

@musale musale merged commit fc4f1e0 into next/fluentui Apr 25, 2023
@musale musale deleted the fix/theme/person branch April 25, 2023 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Update theming for mgt-person
4 participants