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

Added ellipses for long attachment names #6294

Merged
merged 5 commits into from
Nov 30, 2021

Conversation

akshayasalvi
Copy link
Contributor

@akshayasalvi akshayasalvi commented Nov 14, 2021

Details

  • Added ellipses for long attachment names so that the text doesn't overflow and file type is known

Fixed Issues

$ #6157

Tests

  1. Tested with long file names
  2. Tested with small file names
  3. Tested the impact in the chat list

QA Steps

  1. Go to any chat and click on "Add Attachment"
  2. Select a non-image file with a very long name
  3. The attachment preview shouldn't overflow the UI
  4. File-name should be wrapped into multiple line text.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

web-long-attachment-name

Mobile Web

mweb-long-attachment-name

Desktop

desktop-long-attachment-name

iOS

ios-long-attachment-name

Android

android-long-attachment-name

@akshayasalvi
Copy link
Contributor Author

@Luke9389 While the ellipses mode is working fine for the AttachmentModal, the same view is used in the chats as well where it goes into a multiline view.

image

This will also get updated to a single line ellipses in the text. Works right?

@akshayasalvi akshayasalvi marked this pull request as ready for review November 29, 2021 18:30
@akshayasalvi akshayasalvi requested a review from a team as a code owner November 29, 2021 18:30
@botify botify requested a review from Luke9389 November 29, 2021 18:30
@MelvinBot MelvinBot removed the request for review from a team November 29, 2021 18:30
@akshayasalvi
Copy link
Contributor Author

@Luke9389 Based on the current view of the messages I've gone ahead and wrapped the content instead of adding ellipses. I will wait for your comments if you feel we should go back to our ellipses approach only.

@Luke9389
Copy link
Contributor

Ok, I'm fine with this. Could you change the name of the PR as well, please?

@Luke9389 Luke9389 merged commit ff64eb0 into Expensify:main Nov 30, 2021
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

OSBotify commented Dec 7, 2021

🚀 Deployed to staging by @Luke9389 in version: 1.1.17-8 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

OSBotify commented Dec 8, 2021

🚀 Deployed to production by @roryabraham in version: 1.1.18-3 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

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.

None yet

3 participants