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

New stories intro assets and links #15825

Merged
merged 1 commit into from
Feb 9, 2021
Merged

Conversation

bjtitus
Copy link
Contributor

@bjtitus bjtitus commented Feb 8, 2021

Adds new assets and links for the Stories intro screen.

The iOS counterpart of wordpress-mobile/WordPress-Android#13526

Testing

  • Freshly install the WordPress app (or delete the storiesIntroWasAcknowledged User Default).
  • Tap the FAB from your Site Details screen.
  • Select the "Story post" post option.
  • Ensure that tapping the demo stories (shown below) opens and loads the demo stories.
  • Ensure the images below are shown.

Screen Shot 2021-02-07 at 9 56 42 PM

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@bjtitus bjtitus added this to the 16.7 milestone Feb 8, 2021
@bjtitus bjtitus requested a review from guarani February 8, 2021 04:57
@bjtitus bjtitus self-assigned this Feb 8, 2021
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Feb 8, 2021

You can test the changes on this Pull Request by downloading it from AppCenter here with build number: 41896. IPA is available here. If you need access to this, you can ask a maintainer to add you.

@peril-wordpress-mobile
Copy link

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

Hi @bjtitus, I took this for spin and it seems to work and the images now load for the two demo stories.

There's a few things I've noticed, but since I'm not familiar with the project or whether there are some know issues I'll list here everything I came across (some might not correspond to this PR, not sure).

Red screen

Although the first time I tapped "Story post" on the FAB it worked, the second time I got a blank red screen (not the React Native red error screen, just a plain red screen):

See screen recording

Pixelated images

  • The small blue WordPress icon on the top-left corner of the demo stories screens is pixelated on an iPhone 11
  • This demo screen doesn't look high enough resolution for the iPhone 11 I was using, especially noticeable on this screen that shows image selection:
See screenshot

Android back buttons in demo content

This one is a bit subjective but the demo story shows and Android screen (back buttons, etc) which stand out IMO when using the app on an iPhone

See screenshot

Loading flicker

As the demo stories load, there's some flicker as a dark gray loading screen is shown (maybe using a lighter-colored loading screen would reduce flicker). I understand if this is content loaded from the web, but thought I'd mention it:

See screen recording

Here's a few frames during loading that show what looks to be causing the flicker:

White screen Small black flicker Grey loading screen

Loading indicator

I'm not sure if this is a bug, but the loading indicator step for the final image in the story doesn't load (not sure if that's expected):

See screenshot

Barring the red screen, the rest don't really look like blockers for this PR (let me know though if the red screen is not a blocker for this PR either and I can approve this).

@bjtitus
Copy link
Contributor Author

bjtitus commented Feb 8, 2021

Thanks @guarani!

the second time I got a blank red screen

This is expected. The Stories editor screen hasn't been merged in yet and the red is a placeholder.

Pixelated images

I believe the favicon issue is a bug on web. It looks like we should be requesting a larger image size at 2x like we do in some other places.

I'm checking to see if the stories content image is pixelated because of the 2x scaling again, or if it's just the compression from the export on Android.

Android back buttons in demo content

This is expected for now. We should probably consider adding iOS specific demos.

Loading indicator

This seems like a bug. I'll follow up with @aforcier to see if we are tracking it on web.

@guarani guarani self-requested a review February 8, 2021 17:23
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

Thanks @guarani!

the second time I got a blank red screen

This is expected. The Stories editor screen hasn't been merged in yet and the red is a placeholder.

Ok, great! I wasn't sure what stage this was at so wasn't sure if it was expected or not – thanks for the context!

Pixelated images

I believe the favicon issue is a bug on web. It looks like we should be requesting a larger image size at 2x like we do in some other places.

I'm checking to see if the stories content image is pixelated because of the 2x scaling again, or if it's just the compression from the export on Android.

👍

Android back buttons in demo content

This is expected for now. We should probably consider adding iOS specific demos.

No worries.

Loading indicator

This seems like a bug. I'll follow up with @aforcier to see if we are tracking it on web.

Thanks! The loading flicker I mentioned above might also be a bug.

@jkmassel
Copy link
Contributor

jkmassel commented Feb 8, 2021

👋 We're freezing 16.7 today, so this PR is being bumped to 16.8. If you need this to be part of the 16.7 release, please merge it into the release/16.7 branch and DM me – I'll be happy to cut a new beta release!

@jkmassel jkmassel modified the milestones: 16.7, 16.8 Feb 8, 2021
@aforcier
Copy link
Contributor

aforcier commented Feb 9, 2021

Thanks for the detailed issue reports @guarani! I have some notes on some of the web-related stuff:

The small blue WordPress icon on the top-left corner of the demo stories screens is pixelated on an iPhone 11

That one's a known issue that was resolved in the past but seems to have reappeared (at least for WordPress.com sites). It's logged and I'll look into it.

This demo screen doesn't look high enough resolution for the iPhone 11 I was using, especially noticeable on this screen that shows image selection:

Looking at the original uploaded video that's being played in that slide, it looks like that's just the upload quality of the video and not a playback issue.

As the demo stories load, there's some flicker as a dark gray loading screen is shown (maybe using a lighter-colored loading screen would reduce flicker). I understand if this is content loaded from the web, but thought I'd mention it:

This one's a known issue - we've mostly been seeing it on Mobile Chrome, but I added a note that it's happening on Safari as well.

I'm not sure if this is a bug, but the loading indicator step for the final image in the story doesn't load (not sure if that's expected):

I wasn't able to reproduce this, the indicator is working correctly for me throughout both stories for this PR (note that it's a progress bar not a loading indicator, and in the case of videos will match the length of the video - the screenshot shared is at the very start of the video, where no progress would be showing yet, I'm not sure if that accounts for what you're seeing or if there's more going on @guarani ).

@guarani
Copy link
Contributor

guarani commented Feb 9, 2021

The small blue WordPress icon on the top-left corner of the demo stories screens is pixelated on an iPhone 11

That one's a known issue that was resolved in the past but seems to have reappeared (at least for WordPress.com sites). It's logged and I'll look into it.

Cool, thanks @aforcier!

This demo screen doesn't look high enough resolution for the iPhone 11 I was using, especially noticeable on this screen that shows image selection:

Looking at the original uploaded video that's being played in that slide, it looks like that's just the upload quality of the video and not a playback issue.

Ok

As the demo stories load, there's some flicker as a dark gray loading screen is shown (maybe using a lighter-colored loading screen would reduce flicker). I understand if this is content loaded from the web, but thought I'd mention it:

This one's a known issue - we've mostly been seeing it on Mobile Chrome, but I added a note that it's happening on Safari as well.

🙇

I'm not sure if this is a bug, but the loading indicator step for the final image in the story doesn't load (not sure if that's expected):

I wasn't able to reproduce this, the indicator is working correctly for me throughout both stories for this PR (note that it's a progress bar not a loading indicator, and in the case of videos will match the length of the video - the screenshot shared is at the very start of the video, where no progress would be showing yet, I'm not sure if that accounts for what you're seeing or if there's more going on @guarani ).

True, it is a progress bar :) I tried it again today and couldn't reproduce on that slide. But there were other slides on both demo stories that sometimes didn't play at first (perhaps it was an internet issue) and after going back and forth between the slides, in some cases they eventually played.

@bjtitus bjtitus merged commit 19c662c into develop Feb 9, 2021
@bjtitus bjtitus deleted the stories/update-intro-assets branch February 9, 2021 18:52
@aforcier aforcier mentioned this pull request Feb 17, 2021
3 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.

4 participants