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

[Bug] 3rd screen of Flag content flashes and disappears #1145

Closed
setch-l opened this issue May 16, 2024 · 11 comments
Closed

[Bug] 3rd screen of Flag content flashes and disappears #1145

setch-l opened this issue May 16, 2024 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@setch-l
Copy link

setch-l commented May 16, 2024

Steps to Reproduce:

  1. Open Nos on a mac
  2. Open a post to flag the content for
  3. Click on the 3 dots ment
  4. Select other
    Issue: the Send to Nos or Post publicly dialogue flashes for a second and then disappears
    Expected: It should remain on the screen until the user takes action

This slack message has a video of the issue: https://planetary-app.slack.com/archives/CM4EPK324/p1715816908047509

@setch-l setch-l converted this from a draft issue May 16, 2024
@setch-l setch-l added the bug Something isn't working label May 16, 2024
@setch-l setch-l moved this from Backlog to Sprint in Nos Product Board May 21, 2024
@setch-l
Copy link
Author

setch-l commented Jun 3, 2024

@dcadenas @mplorentz - Did someone fix this? I no longer see it happening in Staging?

@dcadenas
Copy link
Contributor

dcadenas commented Jun 3, 2024

I didn't

@mplorentz
Copy link
Member

I didn't, and I don't recall seeing a PR for it.

@setch-l
Copy link
Author

setch-l commented Jun 4, 2024

@joshuatbrown - Did you by chance fix this bug? I'm not seeing it any more on mac and I could have sworn I tested this at some point in the past couple of weeks.

@joshuatbrown
Copy link
Contributor

@setch-l No, I didn't. I thought it happened for me the first time I try to report, but not again after that. Maybe that's what's going on here? I'd like to have one of us investigate and see if we can reproduce since I'm pretty sure I saw it again recently.

@setch-l setch-l moved this from Sprint to UAT in Nos Product Board Jun 4, 2024
@setch-l setch-l moved this from UAT to In progress in Nos Product Board Jun 10, 2024
@dcadenas dcadenas moved this from In progress to Code Review in Nos Product Board Jun 13, 2024
@mplorentz
Copy link
Member

I can't reproduce this on Staging 0.1.18 (325). Moving along to UAT.

@mplorentz mplorentz moved this from Code Review to UAT in Nos Product Board Jun 18, 2024
@setch-l setch-l moved this from UAT to QA in Nos Product Board Jun 18, 2024
@mplorentz
Copy link
Member

mplorentz commented Jun 20, 2024

I am reproducing this intermittently on Staging 0.1.8 (327). Here are the steps I'm using:

  1. Do a fresh install of Nos
  2. Create a new key or log in
  3. On the discover tab, open the profile of a user you have never opened before
  4. Quickly tap the ellipsis in the navigation bar or on a note and choose Flag this Content -> Other
    About 30% of the time the "Send to Nos or flag publicly" dialog disappears. If you try a second time the menu will not disappear.

It seems like some SwiftUI view is getting redrawn or some piece of state is being reset when some data finishes loading in the background. @setch-l I'm moving this back from UAT into the sprint, but feel free to adjust.

@joshuatbrown
Copy link
Contributor

joshuatbrown commented Aug 13, 2024

After maybe too many hours of investigation, I think this is an issue with the way we're using SwiftUI and specifically its confirmation dialog (also known as an action sheet). As I understand it, it’s designed to be used for a single list of options where the user selects one, and then it disappears. Then SwiftUI shows a new sheet with the next set of options. I believe that sometimes it just doesn't show the next sheet because it's not really designed to them in series like this. I'm not confident we can fix this -- I've tried a number of ways and haven't found anything that works yet.

We're using sheets to build a wizard of sorts -- a series of screens through which the user navigates, ending with sending the report. I'd suggest we do something more like what Apple does in Maps with the "Report an Issue" feature, where they present a single sheet, the user selects an option, then continues down the screen selecting more options or entering text. Finally, the user can tap "Send" to send the report.

RPReplay_Final1723564748.MP4

For our case, we could do something like this:

  1. When the user taps the three-dot button, show the sheet of options with "Copy User ID", etc. (the same way we do today)
  2. When the user taps "Flag this user", show a full-screen view where the user selects the category
  3. After they select the category, allow them to select "Send to Nos" or "Flag Publicly"
  4. After they select one of the two options for sending, confirm (maybe even with an action sheet or alert)

Technical Notes

In ReportMenuModifier, our confirmationDialogState is sometimes set to nil. We don't set it to nil anywhere, so it seems like SwiftUI is doing it whenever the user selects an option. That makes sense since it gets dismissed when the user selects an option before the next confirmation dialog is displayed.

After the user selects a category (such as "Impersonation"), we set confirmationDialogState to a non-nil value. When this happens, confirmationDialogState is set to nil before it's set to the non-nil value in processUserSelection. Usually, the previous confirmation dialog is dismissed, then the next one is displayed and stays there, waiting for the user to select an option.

But sometimes that confirmation dialog appears then disappears. In those cases, something is setting confirmationDialogState to nil, just like when things are working properly. The difference when it's broken is that it never gets set to our non-nil value.

Is this a race condition? Maybe. Is there a way to fix it? I don't know. I spent a lot of time debugging, logging, and trying different approaches. I upgraded swiftui-navigation to the latest 2.0.3. I removed the confirmReport state var to eliminate that as a potential source of the issue. You can see my latest work in the branch called eliminate-invalid-report-menu-state.

I think Matt is right:

It seems like some SwiftUI view is getting redrawn or some piece of state is being reset when some data finishes loading in the background.

I'm just not sure how we can avoid it. I think the design isn't quite right anyway, so perhaps we can revisit that and make bigger changes to the code that'll eliminate the problem.

Or maybe I'm just missing something easy and obvious, and someone else can make a one-line change that fixes this. 😀

Update: After discussing all of this with Martin, he suggested that we could try eliminating our dependency on swiftui-navigation for this to determine whether this bug is in that package, rather than in SwiftUI.

Other things I tried:

  • Moving .reportMenu($showingReportMenu, reportedObject: .author(author)) from the ProfileView Button in the navigation bar up to the VStack in the body didn't help
  • Removing the Task { wrappers around setting the confirmationDialogState in ReportMenu just breaks everything (the dialog never appears)

@joshuatbrown
Copy link
Contributor

joshuatbrown commented Aug 13, 2024

@Chardot See my comment above (you can skip all the "Technical Notes"). Sounds like we'll be talking about this next week.

@joshuatbrown joshuatbrown moved this from In progress to UAT in Nos Product Board Aug 13, 2024
@Chardot
Copy link

Chardot commented Aug 15, 2024

After maybe too many hours of investigation, I think this is an issue with the way we're using SwiftUI and specifically its confirmation dialog (also known as an action sheet). As I understand it, it’s designed to be used for a single list of options where the user selects one, and then it disappears. Then SwiftUI shows a new sheet with the next set of options. I believe that sometimes it just doesn't show the next sheet because it's not really designed to them in series like this. I'm not confident we can fix this -- I've tried a number of ways and haven't found anything that works yet.

We're using sheets to build a wizard of sorts -- a series of screens through which the user navigates, ending with sending the report. I'd suggest we do something more like what Apple does in Maps with the "Report an Issue" feature, where they present a single sheet, the user selects an option, then continues down the screen selecting more options or entering text. Finally, the user can tap "Send" to send the report.

RPReplay_Final1723564748.MP4
For our case, we could do something like this:

  1. When the user taps the three-dot button, show the sheet of options with "Copy User ID", etc. (the same way we do today)
  2. When the user taps "Flag this user", show a full-screen view where the user selects the category
  3. After they select the category, allow them to select "Send to Nos" or "Flag Publicly"
  4. After they select one of the two options for sending, confirm (maybe even with an action sheet or alert)

Technical Notes

In ReportMenuModifier, our confirmationDialogState is sometimes set to nil. We don't set it to nil anywhere, so it seems like SwiftUI is doing it whenever the user selects an option. That makes sense since it gets dismissed when the user selects an option before the next confirmation dialog is displayed.

After the user selects a category (such as "Impersonation"), we set confirmationDialogState to a non-nil value. When this happens, confirmationDialogState is set to nil before it's set to the non-nil value in processUserSelection. Usually, the previous confirmation dialog is dismissed, then the next one is displayed and stays there, waiting for the user to select an option.

But sometimes that confirmation dialog appears then disappears. In those cases, something is setting confirmationDialogState to nil, just like when things are working properly. The difference when it's broken is that it never gets set to our non-nil value.

Is this a race condition? Maybe. Is there a way to fix it? I don't know. I spent a lot of time debugging, logging, and trying different approaches. I upgraded swiftui-navigation to the latest 2.0.3. I removed the confirmReport state var to eliminate that as a potential source of the issue. You can see my latest work in the branch called eliminate-invalid-report-menu-state.

I think Matt is right:

It seems like some SwiftUI view is getting redrawn or some piece of state is being reset when some data finishes loading in the background.

I'm just not sure how we can avoid it. I think the design isn't quite right anyway, so perhaps we can revisit that and make bigger changes to the code that'll eliminate the problem.

Or maybe I'm just missing something easy and obvious, and someone else can make a one-line change that fixes this. 😀

Update: After discussing all of this with Martin, he suggested that we could try eliminating our dependency on swiftui-navigation for this to determine whether this bug is in that package, rather than in SwiftUI.

Other things I tried:

  • Moving .reportMenu($showingReportMenu, reportedObject: .author(author)) from the ProfileView Button in the navigation bar up to the VStack in the body didn't help
  • Removing the Task { wrappers around setting the confirmationDialogState in ReportMenu just breaks everything (the dialog never appears)

@joshuatbrown Yes, I also like that design for the content flag wizard 👍 I tried it in Maps for macOS and it centers the wizard menu on the screen (unlike iOS where it is a bottom-sliding sheet).

@setch-l
Copy link
Author

setch-l commented Sep 17, 2024

We ended up redesigning the workflow to use a different format.

@setch-l setch-l closed this as completed Sep 17, 2024
@github-project-automation github-project-automation bot moved this from QA to Done in Nos Product Board Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

5 participants