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

[PS UI] FaviconAvatar component #4414

Merged
merged 17 commits into from
Jun 24, 2022
Merged

[PS UI] FaviconAvatar component #4414

merged 17 commits into from
Jun 24, 2022

Conversation

jpcloureiro
Copy link
Contributor

@jpcloureiro jpcloureiro commented May 26, 2022

Description

FaviconAvatar component implementation, for the Permission System UI.

Default
image

With Error
image

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

Screenshots/Recordings

If applicable, add screenshots or recordings to visualize the changes

Issue

Progresses #4104

@jpcloureiro jpcloureiro requested a review from a team as a code owner May 26, 2022 17:44
@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@Cal-L Cal-L added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Jun 23, 2022
Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

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

One tiny nit and one comment about adding a test for the error state otherwise smashed it out of the park 💯

  • verified component code to the best of my ability
  • verified unit tests (need to update snapshots otherwise working)
  • verified all stories in storybook
  • verified all knobs and knob options work in storybook
  • verified markdown documentation

@georgewrmarshall
Copy link
Contributor

georgewrmarshall commented Jun 24, 2022

@Akatori-Design checkout current fallback idea for this component. Also in description of this PR

Screen Shot 2022-06-24 at 10 55 19 AM

Cal-L and others added 3 commits June 24, 2022 12:03
Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@gantunesr gantunesr left a comment

Choose a reason for hiding this comment

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

LGTM

@Cal-L Cal-L merged commit da76f7d into main Jun 24, 2022
@Cal-L Cal-L deleted the feat/redesign-favicon-avatar branch June 24, 2022 20:38
@github-actions github-actions bot locked and limited conversation to collaborators Jun 24, 2022
@Cal-L Cal-L added release-5.4.0 PRs for v5.4.0 release and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Jul 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-5.4.0 PRs for v5.4.0 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants