Skip to content

Blackscreen Fix Attempt: Move Hint Text State Into a Hook#10343

Closed
charleyf wants to merge 4 commits intomainfrom
charley/move-selfie-feedback-state
Closed

Blackscreen Fix Attempt: Move Hint Text State Into a Hook#10343
charleyf wants to merge 4 commits intomainfrom
charley/move-selfie-feedback-state

Conversation

@charleyf
Copy link
Contributor

@charleyf charleyf commented Mar 29, 2024

🛠 Summary of changes

This is an attempt to fix the blackscreen issue that users are seeing.

  • I noticed that when you click front/back, we render the AcuantCaptureCanvas twice before the permissions prompt, but it does not get rerendered after the permissions prompt is accepted.
  • In main the AcuantSelfieCaptureCanvas component gets rendered twice, then once more after the permissions prompt.

This work removes the extra rerender of AcuantSelfieCaptureCanvas after the permissions prompt under the theory that something about that re-render is leading to the black screen problem.

Note: This is generally a better place to keep this state, so even if it doesn't fix the bug, it still improves the codebase.

📜 Testing Plan

I don't have a way to reliably reproduce the black screen problem as users see it, but you can check that this code does in fact remove a render by:

  • Check out main
  • Add a console.log('render AcuantSelfieCaptureCanvas) to that component, under the const { t } = useI18n();
  • Go to the front/back/selfie page. Click on the selfie - Don't "Allow "camera permissions yet!
  • Note that you should have two console logs.
  • Now, "Allow" the camera permissions.
  • Note you should have three console logs.
  • Repeat these steps on this branch, note that you don't get the third console log.
  • Bonus: You can add a console log in AcuantCaptureCanvas to confirm that this looks more like that component now.

Copy link
Contributor

@night-jellyfish night-jellyfish left a comment

Choose a reason for hiding this comment

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

I tested this out in a few ways:

  1. Following the instructions on this PR (all worked as expected)
  2. Trying to recreate the black screen and then see if I couldn't recreate it in this branch. I could recreate it in this branch, however I'm not even close to positive that the way I recreate the black screen is what is happening when a user encounters it.

I was going to approve it, but I found hint text disappeared in this branch again, even though it seems to have the fix from #10339. I did also see a lint failure (and a test failure, though I think the test is a flaky one and nothing to do with this branch).

Once the hint text and lint issues are fixed I would feel more comfortable approving.

@charleyf
Copy link
Contributor Author

charleyf commented Apr 9, 2024

I'm closing this since it's not obviously the source of the issue and it's causing issues in test. Ideally I'll revisit and move the text into a context, but that's a different PR.

@charleyf charleyf closed this Apr 9, 2024
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.

2 participants