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

fix(mobile): no preview for avatar at header action #3066

Merged
merged 2 commits into from
Mar 13, 2025

Conversation

hyoban
Copy link
Member

@hyoban hyoban commented Mar 13, 2025

Description

PR Type

  • Feature
  • Bugfix
  • Hotfix
  • Other (please describe):

Screenshots (if UI change)

Demo Video (if new feature)

Linked Issues

Additional context

Changelog

  • I have updated the changelog/next.md with my changes.

Copy link

vercel bot commented Mar 13, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
follow ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 13, 2025 3:34am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
follow-external-ssr ⬜️ Ignored (Inspect) Visit Preview Mar 13, 2025 3:34am

@follow-reviewer-bot
Copy link

Suggested PR Title:

feat(avatar): add optional noPreview prop to UserAvatar

Change Summary:
Added optional 'noPreview' prop to UserAvatar component, allowing direct rendering of image without preview gallery. Updated usage in HomeLeftAction accordingly.

Code Review:

Code Review: Change Requests

  1. File: apps/mobile/src/components/ui/avatar/UserAvatar.tsx
    Lines: 21-44
    Issue: The proxy property usage in the Image component is unclear. It appears to be a custom property, and there is no validation or documentation provided for its significance. If this property is essential, it would be helpful to either validate its values or document its purpose clearly to avoid misconfiguration or unexpected runtime behavior.
    Change Request: Add validation or comments documenting the purpose and expected format of the proxy property if necessary.

  2. File: apps/mobile/src/components/ui/avatar/UserAvatar.tsx
    Lines: 52-54
    Issue: The Galeria.Image usage now relies on the imageContent variable, which wraps the Image component. This replacement changes how the component is rendered but does not appear to account for any fallbacks or error handling in scenarios where imageContent might fail to load properly. Furthermore, if noPreview is set, certain aspects related to Galeria are bypassed entirely. Depending on the application's requirements, this may be an oversight.
    Change Request: Ensure appropriate fallback or error handling mechanisms are in place for the imageContent variable if an image fails to render. Reevaluate if the logic governing noPreview and Galeria interactions meets application expectations.

  3. File: apps/mobile/src/modules/screen/action.tsx
    Lines: 44
    Issue: The addition of the noPreview property may affect user experience depending on the use case of the component. If disabling preview functionality (noPreview) impacts user interaction with avatars or changes the component's behavior significantly, this should be tested or verified as part of the pull request.
    Change Request: Ensure there is adequate testing or documentation verifying the desired behavior with noPreview enabled. If this change has user experience implications, confirm that it aligns with stakeholder or design team expectations.


Ensure these issues are addressed before merging the pull request.

@hyoban hyoban enabled auto-merge (squash) March 13, 2025 03:13
@hyoban hyoban merged commit 2c64d28 into dev Mar 13, 2025
11 checks passed
@hyoban hyoban deleted the fix/no-preview-avatar-03-13 branch March 13, 2025 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant