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

Mobile compatible chat #2298

Merged

Conversation

KarimAl-Rashdan
Copy link
Collaborator

@KarimAl-Rashdan KarimAl-Rashdan commented Sep 28, 2023

Fixes #2269

Description

Adds mobile update to volunteer chat feature.
Screen Shot 2023-09-28 at 8 51 25 PM

Screen Shot 2023-09-28 at 8 50 13 PM Screen Shot 2023-09-28 at 8 51 00 PM

Checklist:

  • I have manually tested my changes on desktop and mobile
  • The test suite passes locally with my changes
  • If my change is a UI change, I have attached a screenshot to the description section of this pull request
  • My change is 300 lines of code or less, or has a documented reason in the description why it’s longer
  • I have made any necessary changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • My PR is labeled [WIP] if it is in progress

Copy link
Collaborator

@crayolakat crayolakat left a comment

Choose a reason for hiding this comment

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

Hi Karim, thank you for submitting this PR! I just have 1 question:

Is there a reason why some blocks/lines of code are commented out instead of deleted?

src/components/AssignmentTexter/StyleControls.js Outdated Show resolved Hide resolved
src/components/AssignmentTexter/StyleControls.js Outdated Show resolved Hide resolved
Delete flexBasis styling
Delete comment on height
@crayolakat crayolakat added the S-ready for stage-main (qa) Status (ADMINS ONLY): PR label for those ready to be added for stage: Approved, tests, etc label Oct 4, 2023
@crayolakat crayolakat self-requested a review October 4, 2023 16:58
@crayolakat crayolakat added S-testing-on-stage Status (ADMINS ONLY): PR label for presence on a staging instance and removed S-ready for stage-main (qa) Status (ADMINS ONLY): PR label for those ready to be added for stage: Approved, tests, etc labels Oct 9, 2023
@crayolakat crayolakat mentioned this pull request Oct 9, 2023
Copy link
Collaborator

@Arique1104 Arique1104 left a comment

Choose a reason for hiding this comment

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

Notes from Community Coalition QA
TLDR;

  • On a cell phone, after a volunteer opts out a phone number within a texting campaign, can the back arrow on the top half of the screen successfully override the opt-out and allow the volunteer to return back to an opted-out phone number and successfully send a text message?
  • If the above is true, can we resolve this by removing the second bar from the top and ensuring that a functional home button and skip button already provide the back and forth functionality we would lose by removing those arrows?
  • Is there functionality that we lose when getting rid of that second bar? If so, what would we want to keep?

Notes Continued:

  • These changes work well, although we hit some interesting moments during the QA for some edge cases.
  • Some how, between using the opt-out button, we lose the ability to use the <- arrow at the top of the screen.
  • And when pressed back enough times, it does look like our Spoke instance will permit to sending a text to someone who has already been opted-out.
  • We would need to try this on a production texting framework to confirm that this only occurs in development because we are using fake data. Would like to run a live QA on a production site to validate this edge case concern.
  • We have to check and see if the "All Responses" tab actually scrolls down if it contains 20+ canned responses within the campaign.
  • Esthetically we are deciding if the Second Tab from the top of the screen with the <- and -> buttons is strategically necessary when there is a skip button and a home button to return to text messages that received responses. Removing that second box from the top (or make it more useful) would help with the back-arrow opt-out bug we found in QA. Removing those back and forth arrow and simply keeping the "skip" button would protect our functionality and rid us of the opt-out bug.
  • @crayolakat, what would you suggest we prioritize? It's difficult to predict if this is a real issue in production without a live API key to test it. Is this something that Travis can include as part of his QA? Checking the mobile feature update for these edge cases?

@crayolakat
Copy link
Collaborator

Hi @Arique1104, I tested this on fake data on MoveOn's staging website, and was not able to replicate the issue with the <- arrow or being able to text someone who is already opted out.

Also, I verified that the "All Responses" tab scrolls down if it contains 20+ canned responses within the campaign.

I will coordinate with you further on Slack

@Arique1104
Copy link
Collaborator

After careful review in our Staging Branch, we were able to confirm that no new bugs were introduced to the code base with these changes.

A bug that currently exists in main is now reported correctly here.

This new mobile update functionality will now be part of the Spoke codebase. Thank you so much for your work on this @KarimAl-Rashdan!

@crayolakat crayolakat merged commit a22c2ea into StateVoicesNational:main Oct 27, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-testing-on-stage Status (ADMINS ONLY): PR label for presence on a staging instance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mobile-Only Update
3 participants