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

feat: Added a Ghost UI screen #7746

Closed
wants to merge 11 commits into from

Conversation

mananjadhav
Copy link
Collaborator

@mananjadhav mananjadhav commented Feb 14, 2022

Details

  • Adds a background screen with shimmer animation, when the chat is loading
  • It also adds an additional dependency react-content-loader for SVG based animation on components.

Fixed Issues

$ #7081

Tests

  • Verify that no errors appear in the JS console

QA Steps

  • Verify that no errors appear in the JS console

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@mananjadhav
Copy link
Collaborator Author

@Luke9389 @parasharrajat, can you help me verify the loader condition. It seems that what I am using for the initial load is incorrect.

I have attached two screens for Ghost UI on Mobile as well as Web for reference.
https://user-images.githubusercontent.com/3069065/155564261-46fbbdcc-50ea-4a0a-92ef-84927b1680f3.mov

ios-screen

But when it transitions from Ghost screen to the chats, it switches one chat at a time, so I am guessing I am using an incorrect flag for the loader. Following is the attached ref video.

transition-bug.mov

@parasharrajat
Copy link
Member

parasharrajat commented Feb 24, 2022

This Onyx key is updated multiple times during the app cycle. So maybe just create a state and update that only once when this is true. I don't know much about this.

@mananjadhav mananjadhav closed this Mar 8, 2022
@Luke9389
Copy link
Contributor

Luke9389 commented Mar 9, 2022

@mananjadhav did you close this intentionally? If so would you be willing to post your reason (can be brief). Just easier for everyone to follow. I see there's another PR. You can always link to that here as well.

Other PR: #8042

@mananjadhav
Copy link
Collaborator Author

Yeah @Luke9389. I closed the old PR because it didn’t have the new PR template checklist. Raised a fresh one with the same changes here #8042

@mananjadhav mananjadhav mentioned this pull request May 2, 2022
50 tasks
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