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

Chore: Refactor text preview for MediaView #1714

Merged
merged 1 commit into from
Feb 27, 2025
Merged

Conversation

hjiangsu
Copy link
Member

Pull Request Description

This PR is one of a few refactors I'll be doing to the general MediaView widget. Right now, the MediaView widget is a bit complicated and can be improved by splitting it up into smaller widgets, each handling a different type of media (e.g., image, video, text, link).

Since MediaView is used pretty much everywhere (e.g., feed pages, account page, post page, etc.), refactoring this widget to be easier to maintain and potentially more performant would be good!

For this specific PR, I've extracted out the text portion of MediaView into its own widget (MediaViewText), and moved MediaView into the /shared/media directory. I've also cleaned up the MediaViewText widget a little bit!

Issue Being Fixed

Issue Number: N/A

Screenshots / Recordings

Checklist

  • If a new package was added, did you ensure it uses an appropriate license and is actively maintained?
  • Did you use localized strings (and added appropriate descriptions) where applicable?
  • Did you add semanticLabels where applicable for accessibility?

Copy link
Member

@micahmo micahmo left a comment

Choose a reason for hiding this comment

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

Very nice!

@hjiangsu hjiangsu merged commit 87a2ac3 into develop Feb 27, 2025
1 check passed
@hjiangsu hjiangsu deleted the feat/media-view-text branch February 27, 2025 18:59
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.

2 participants