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.

Sign up for GitHub

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

Jump to bottom

Trim long workspace name #4835

Merged
merged 2 commits into from
Aug 25, 2021
Merged

Trim long workspace name #4835

merged 2 commits into from
Aug 25, 2021

Conversation

akshayasalvi
Copy link
Contributor

@akshayasalvi akshayasalvi commented Aug 25, 2021 •
edited
Loading

@Julesssss Can you please review this?

Details

  • Trimmed workspace name when its too long

Fixed Issues

$ #4747

Tests

  1. Test on WorkspaceSidebar
  2. Tested on Home Sidebar

QA Steps

  1. Create an account with a very long email
  2. Create a new workspace
  3. Then visit the workspace and verify that it doesn't get trimmed outside the container.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Screenshot 2021-08-25 at 11 17 34 PM

Screenshot 2021-08-25 at 11 01 01 PM

Mobile Web

Screenshot 2021-08-25 at 11 36 02 PM

Screenshot 2021-08-25 at 11 35 55 PM

Desktop

Screenshot 2021-08-25 at 11 38 02 PM

Screenshot 2021-08-25 at 11 37 48 PM

iOS

Screenshot 2021-08-25 at 11 46 04 PM

Screenshot 2021-08-25 at 11 46 17 PM

Android

Screenshot 2021-08-26 at 12 05 52 AM

Screenshot 2021-08-26 at 12 05 26 AM

@akshayasalvi akshayasalvi requested a review from a team as a code owner August 25, 2021 18:36
@MelvinBot MelvinBot requested review from aldo-expensify and removed request for a team August 25, 2021 18:36
aldo-expensify
aldo-expensify approved these changes Aug 25, 2021
View reviewed changes
Copy link
Contributor

@aldo-expensify aldo-expensify left a comment

Choose a reason for hiding this comment

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

Looks fine, tested in web / android.

@aldo-expensify aldo-expensify merged commit 2efff1b into Expensify:main Aug 25, 2021
@OSBotify OSBotify mentioned this pull request Aug 25, 2021
Closed
@isagoico
Copy link

Hello @akshayasalvi, Any QA tests needed for this PR?

@akshayasalvi
Copy link
Contributor Author

Yes, @isagoico. QA is needed.

@Julesssss
Copy link
Contributor

Julesssss commented Aug 26, 2021 •
edited
Loading

Looking good. I have one outstanding question which I asked here on the issue.

@Julesssss
Copy link
Contributor

Oh this was already merged... 😄

This was referenced Aug 29, 2021
Closed
Merged
@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.0.88-2 🚀

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

@botify
Copy link

botify commented Sep 1, 2021

This has been deployed to production and is now subject to a 7-day regression period.
If no regressions arise, payment will be issued on 2021-09-08. 🎊

@mallenexpensify
Copy link
Contributor

@akshayasalvi when creating new PRs, can you please include the full link to the GH issue and not just the issue number?
$ https://github.com/Expensify/App/issues/4747 instead of
2021-09-06_10-24-01 (34)

@akshayasalvi
Copy link
Contributor Author

Okay noted. Will do.

@luacmartins luacmartins mentioned this pull request Jan 6, 2023
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aldo-expensify aldo-expensify aldo-expensify approved these changes

Assignees

@akshayasalvi akshayasalvi

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@akshayasalvi @isagoico @Julesssss @OSBotify @botify @mallenexpensify @aldo-expensify