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

CARDS-1741: Add a "PageStart" extension point to the patient UI #1180

Open
wants to merge 12 commits into
base: dev
Choose a base branch
from

Conversation

veronikaslc
Copy link
Contributor

@veronikaslc veronikaslc commented Sep 20, 2022

To test CARDS-1741 build and run with ./start_cards.sh --demo -P proms. Open http://localhost:8080/Survey to see demo warning banner.

also fixes CARDS-1929, CARDS-1930, CARDS-1931,

@sashaandjic sashaandjic added bug Something isn't working and removed Test me! Ready for testing labels Sep 20, 2022
@sashaandjic
Copy link
Contributor

sashaandjic commented Sep 20, 2022

The banner is not visible when prems or proms patient logs in.
image
image

Also, should we add banner here too?
image

@sashaandjic
Copy link
Contributor

sashaandjic commented Sep 20, 2022

Steps to reproduce:

  • start cards in either proms or prems mode
  • login as admin
  • create a Patient Info and Visit Info forms --> observe the surveys that are created
  • logout and login as patient
  • when survey form(s) are opened, notice that the demo banner is present but almost completely covered - only small slices on each end are visible

Please note: if in prems mode, you would need to generate a token to login as patient, as patients can only use links (with token) that are going to be mailed to them.

@veronikaslc veronikaslc added Test me! Ready for testing and removed bug Something isn't working labels Sep 21, 2022
Copy link
Contributor

@marta- marta- left a comment

Choose a reason for hiding this comment

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

A couple of layout issues popped up in the Patient UI with the introduction of the Demo banner, and they need to be addressed:

  • On a short screen (e.g. on a smartphone or a short desktop browser window) the footer containing the link to the Terms of Use is pushed too high and ends up being displayed above the action button. It should be positioned at the bottom, like it is when there is no banner.
    image

  • After logging in, when scrolling a survey up, the banner is visible on the sides behind the survey header. It should not be visible at all once the user starts scrolling up.
    image
    image

  • The pagination controls look misplaced when the banner is on:
    image

To replicate all of the above:

  • start with -P proms --demo
  • as admin, create patient info form and visit info form; the latter should have 6012-HC-Congenital Cardiac selected for the Clinic question to make sure there's a paginated survey to fill out
  • from Chrome dev tools, switch to emulating a mobile device (the above screenshots are on emulated iPhone SE), then log in and complete surveys as a patient.

@marta- marta- added bug Something isn't working and removed Test me! Ready for testing labels Sep 21, 2022
@veronikaslc veronikaslc added Test me! Ready for testing and removed bug Something isn't working labels Sep 22, 2022
Copy link
Contributor

@marta- marta- left a comment

Choose a reason for hiding this comment

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

  1. The absolute positioning of the banners does not work well in the default UI (admin and clinician), as the banners get scrolled up leaving an unsightly gap at the top (see screenshot).
    image
    I suggest passing the position as a prop so we can customize it on a case by case basis.
  2. All looks good now in the patient UI with one minor exception: the shadow of the banner visible on the sides of the patient ui header looks a bit messy. I suggest we get rid of that shadow from all banners.
    image
  3. There is still a screen without a banner:
  • In Administration/Patient identification, uncheck the first checkbox ("Patients can answer surveys without a personalized link")
  • Sign out and go to /Survey.html: a page displaying the welcome message and informing the user that "To fill out surveys, please follow the personalized link that was emailed to you." is displayed. There is no banner on that page.
    image

@marta- marta- removed the Test me! Ready for testing label Sep 22, 2022
@sashaandjic
Copy link
Contributor

no banner here too:
image

@sashaandjic
Copy link
Contributor

Cannot access Admin button on narrow screens (probably out of scope)
image

@sashaandjic
Copy link
Contributor

Login page looks weird with both demo and maintenance banners:
image

@marta-
Copy link
Contributor

marta- commented Sep 22, 2022

Cannot access Admin button on narrow screens (probably out of scope)

Indeed it's out of scope.

Login page looks weird with both demo and maintenance banners:

This one is out of scope too, since the login page extension point was developed elsewhere.

These issues should be reported separately.

@marta-
Copy link
Contributor

marta- commented Sep 22, 2022

no banner here too:

The banner is there but it is under the landing page UI. TBH with the current implementation of the landing page it won't be trivial to display it. Additionally, given that the production workflows now exclude staff interacting with the app, this can be considered very low priority and should not condition the merge of this PR. We may even end up redoing the landing page completely, and we'll tackle the banner display then (or after).

@veronikaslc
Copy link
Contributor Author

All should be fixed except reverting to the fixed position did not seem to fix #1 unsightly gap at the top propblem.

@sashaandjic
Copy link
Contributor

sashaandjic commented Sep 23, 2022

small ui issue -not sure if it is related to this PR

Patient.1._.DATA-PRO.-.Google.Chrome.2022-09-23.09-40-27.mp4
DATA-PRO.-.Google.Chrome.2022-09-23.09-45-24.mp4

@sashaandjic
Copy link
Contributor

sashaandjic commented Sep 23, 2022

On iPad and on cell phone patent cannot access the BEGIN button to start surveys.

DATA-PRO.-.Google.Chrome.2022-09-23.09-48-31.mp4
DATA-PRO.-.Google.Chrome.2022-09-23.09-48-06.mp4

@marta-
Copy link
Contributor

marta- commented Sep 23, 2022

small ui issue -not sure if it is related to this PR

It is related to this PR and I wouldn't call it small. The introduction of Collapse here appears to be the cause.

@sashaandjic sashaandjic added bug Something isn't working and removed Test me! Ready for testing labels Sep 26, 2022
@sashaandjic
Copy link
Contributor

Unable to access Admin button

Dashboard._.DATA-PRO.-.Google.Chrome.2022-09-26.13-29-37.mp4

@sashaandjic
Copy link
Contributor

sashaandjic commented Sep 26, 2022

Something sticks out from the background and prevents users from editing
image

@veronikaslc
Copy link
Contributor Author

veronikaslc commented Sep 26, 2022

Unable to access Admin button

Failing to load DashboardViews is most likely out of the scope as this PR does not seem to alter extension loading or DashboardViews in particular. wdt @marta-

Something stick out from the background and prevent users from editing

By some reason can not reproduce that one on my build

@marta-
Copy link
Contributor

marta- commented Sep 26, 2022

Failing to load DashboardViews is most likely out of the scope as this PR does not seem to alter extension loading or DashboardViews in particular. wdt @marta-

Indeed that's a poorly phrased log message. The cause of it is harmless, we simply don't have custom widgets displayed on the clinic dashboard (and we're not supposed to have them). Please ignore it.

Something stick out from the background and prevent users from editing

By some reason can not reproduce that one on my build

I can't replicate it either, but something did make the ghost element show up there at the bottom in that screenshot so I'll keep investigating.

@veronikaslc veronikaslc added question Further information is requested Test me! Ready for testing and removed bug Something isn't working labels Sep 27, 2022
@sashaandjic
Copy link
Contributor

sashaandjic commented Sep 27, 2022

it turns out that issue with the vertical scroll bar on narrow screen is only happening in this PR 1929

@sashaandjic
Copy link
Contributor

image

@veronikaslc
Copy link
Contributor Author

image

@sashaandjic
Copy link
Contributor

Can this be improved?

DATA-PRO.-.Google.Chrome.2022-09-27.12-17-16.mp4

@sashaandjic
Copy link
Contributor

sashaandjic commented Sep 27, 2022

@veronikaslc this one is only happening in this PR
image

here is the video from dev branch:

Dashboard._.DATA-PRO.-.Google.Chrome.2022-09-27.12-22-39.mp4

@veronikaslc
Copy link
Contributor Author

Can this be improved?

DATA-PRO.-.Google.Chrome.2022-09-27.12-17-16.mp4

This should be fixed now.

@veronikaslc veronikaslc removed the question Further information is requested label Sep 27, 2022
@sashaandjic
Copy link
Contributor

sashaandjic commented Sep 28, 2022

All reported defects except this one are now fixed:
https://user-images.githubusercontent.com/23243244/192674977-10ffc256-b634-4467-ba02-dc08ce7e8eab.mp4

Related Jira tasks CARDS-1930 and CARDS-1931 are fixed too.

QUESTION: Should the remaining defect be logged separately so we can close this PR?

@sashaandjic sashaandjic added the question Further information is requested label Sep 28, 2022
@veronikaslc
Copy link
Contributor Author

no banner here too:

The banner is there but it is under the landing page UI. TBH with the current implementation of the landing page it won't be trivial to display it. Additionally, given that the production workflows now exclude staff interacting with the app, this can be considered very low priority and should not condition the merge of this PR. We may even end up redoing the landing page completely, and we'll tackle the banner display then (or after).

The only reply regarding landing page was that one.

@sashaandjic sashaandjic added tested Passed manual testing, needs code review and removed question Further information is requested Test me! Ready for testing labels Sep 28, 2022
Copy link
Contributor

@sashaandjic sashaandjic left a comment

Choose a reason for hiding this comment

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

tested

The remaining issue is low priority and is reported in CARDS-1934

@veronikaslc veronikaslc changed the title CASDS-1741: Add a "PageStart" extension point to the patient UI CARDS-1741: Add a "PageStart" extension point to the patient UI Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested Passed manual testing, needs code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants