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

MM-54199 - attachment author name missing on mobile #7575

Merged
merged 2 commits into from
Oct 6, 2023

Conversation

pvev
Copy link
Contributor

@pvev pvev commented Sep 29, 2023

Summary

There was a small display issue with the author name in the matterpoll attachment in mobile view posts. This PR makes sure to display the author name either the icon is provided or not.

Ticket Link

https://mattermost.atlassian.net/browse/MM-54199

Checklist

  • [X ] Added or updated unit tests (required for all new features)
  • [ X] Has UI changes
  • Includes text changes and localization file updates
  • [X ] Have tested against the 5 core themes to ensure consistency between them.

Device Information

This PR was tested on:

Screenshots

Before:
Screenshot 2023-09-29 at 15 50 11

After:
Screenshot 2023-09-29 at 15 22 04

Release Note

NONE

@pvev pvev requested review from enahum and larkox September 29, 2023 13:52
@pvev pvev added the 2: Dev Review Requires review by a core commiter label Sep 29, 2023
Copy link
Contributor

@larkox larkox left a comment

Choose a reason for hiding this comment

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

Great!

@enahum enahum removed the 2: Dev Review Requires review by a core commiter label Sep 29, 2023
@pvev pvev added the 3: QA Review Requires review by a QA tester label Sep 29, 2023
@pvev
Copy link
Contributor Author

pvev commented Oct 4, 2023

@DHaussermann friendly reminder on this one :) . Thanks!

@DHaussermann
Copy link

/update-branch

@DHaussermann DHaussermann added the Build Apps for PR Build the mobile app for iOS and Android to test label Oct 5, 2023
@mattermost-build mattermost-build removed the Build Apps for PR Build the mobile app for iOS and Android to test label Oct 5, 2023
@mattermost-build
Copy link
Contributor

Building app in separate branch.

@DHaussermann
Copy link

@pvev maybe I've misunderstood something here but, I don't see a change. The authors name is still not visible in the post attachment data when I view it with the mobile client. I also used the Matterpoll example.

image

I tested this on Android (iOS testing is blocked for me for reasons unrelated to this PR) as follows:

  • On community open the test public channel and created a poll with /poll
  • Noted the authors' name shown in the attachment when viewed in the webapp
  • Downloaded this build for PR from the Mobile: Test Builds channel and side-loaded it onto my Android device
  • Launched MM Beta app and connected the app to Community server
  • Observed the post created in test channel above
  • Created a 2nd poll from the mobile app for fun to see if the post would look any different
    Observed: The author is still not shown

I using a Samsung Galaxy S22 running Android 13 with the stock One UI 5.1 launcher.

@DHaussermann
Copy link

Sorry @pvev, we can ignore the comment above ☝️
I don't know how but, I seem to have been having a deployment issue earlier.

After repeating the steps and side-loading the app again I can see your change 🎉
image

Testing is ongoing - I'm still trying to get my iOS device working with test builds. I will post an update on this shortly.

@DHaussermann
Copy link

DHaussermann commented Oct 6, 2023

Tested and passed.
Tested with /poll as it provides the case where there is an author but no icon

  • I see this working as expected.
  • When there is no author_icon the post author is still visible in the post content on mobile
  • Tested on Androis and iOS

LGTM!

Thanks @pvev for helping troubleshoot my deployment issue on this one 👍

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Oct 6, 2023
@pvev pvev merged commit 2e3d497 into main Oct 6, 2023
7 checks passed
@pvev pvev deleted the MM-54199-attachment-author-name-missing-on-mobile branch October 6, 2023 16:09
@amyblais amyblais added this to the v2.10.0 milestone Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants