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 ellipsis in DisplayNames - 2 #6972

Conversation

CamilaRivera
Copy link
Contributor

@CamilaRivera CamilaRivera commented Dec 31, 2021

Details

Add display: inline-block in web to make ellipsis work.

Fixed Issues

$ #6913

Tests | QA Steps

  1. Open app
  2. Create a group with multiple contacts
  3. Check LHN for the group with multiple contacts, it should show an ellipsis if the names don't fit
  4. Create a contact with a long email to check it also shows an ellipsis at the end
  5. Check in the search page the same behavior as LHN

Expected result: You should see the ellipsis for all cases above.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image

Mobile Web

Chrome Android

Screen Shot 2022-01-03 at 7 18 09 PM

Screen Shot 2022-01-03 at 7 17 52 PM

Screen Shot 2022-01-03 at 7 17 30 PM

Safari IOS

Screen Shot 2022-01-03 at 7 47 52 PM

Screen Shot 2022-01-03 at 7 47 46 PM

Screen Shot 2022-01-03 at 7 47 42 PM

Desktop

Screen Shot 2022-01-03 at 7 30 23 PM

Screen Shot 2022-01-03 at 7 30 17 PM

Screen Shot 2022-01-03 at 7 30 11 PM

iOS

Screen Shot 2022-01-03 at 7 45 21 PM

Screen Shot 2022-01-03 at 7 45 14 PM

Screen Shot 2022-01-03 at 7 45 08 PM

Android

Screenshot_20220103-193913
Screenshot_20220103-193922
Screenshot_20220103-193931

@CamilaRivera CamilaRivera requested a review from a team as a code owner December 31, 2021 21:57
@MelvinBot MelvinBot requested review from Beamanator and parasharrajat and removed request for a team December 31, 2021 21:57
@CamilaRivera CamilaRivera changed the title Fix ellipsis in DisplayNames Fix ellipsis in DisplayNames - 2 Dec 31, 2021
@parasharrajat
Copy link
Member

@CamilaRivera Let's find the real cause why this is not working natively. Why RN-web is using multiline styles.

@Beamanator
Copy link
Contributor

Also since you tested on mobile web & desktop would you please provide some screenshots? :D (also for ios and android if you're able to test there)

@CamilaRivera
Copy link
Contributor Author

@CamilaRivera Let's find the real cause why this is not working natively. Why RN-web is using multiline styles.

Not sure what you mean about "multiline styles", but this is what I found:

The base requirements for the ellipsis, according to this SO, are:

  overflow: hidden;
  text-overflow: ellipsis;
  white-space: nowrap;

and a constrained width. We are missing this last part because of the display: -webkit-box currently being set. Haven't found why that is set like that.

@CamilaRivera
Copy link
Contributor Author

Also since you tested on mobile web & desktop would you please provide some screenshots? :D (also for ios and android if you're able to test there)

@Beamanator Updated the description with screenshots

@parasharrajat
Copy link
Member

I meant the same thing display: -webkit-box is multiline styles. So Now I would wait before merging this and request you to do some analysis on finding the root cause. If we are not able to determine then we can proceed with this approach which is a hacky solution.

This problem is coming due to the recent upgrade of RN-web. But is this a bug with RN-web or an issue in our repo?

@parasharrajat
Copy link
Member

Here is the issue with RN-web necolas/react-native-web#2186. Let's wait for the solution

@parasharrajat
Copy link
Member

I have submitted a fix for RN-web necolas/react-native-web#2193 which should be merged soon and then this issue will be fixed.

@Beamanator
Copy link
Contributor

Closing since the fix was made in react-native-web (our fork, for now)

@Beamanator Beamanator closed this Jan 17, 2022
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.

3 participants