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

[#2159] Display the Approve Partner button on the top with a tooltip #2163

Conversation

rhian-cs
Copy link
Contributor

Resolves #2159

Description

In the Approve Partner screen, the Approve Partner button was at the bottom of the page.
Since the page is quite long, it would be interesting if the user could see the button either at the top or while they're scrolling the page.

Two options were suggested by @edwinthinks. The first is placing the button at the top of the page, beside the "Application Details" header. However, this may encourage the user to not read the application instead and just click Approve. The other option would be to add a fixed button that scrolls with the user, which is what I did.

To test the changes I also added a fixture to help mock the Partner API.

Type of change

  • Minor adjustment (non-breaking change which fixes an non-bug issue)

How Has This Been Tested?

Manual testing:

  • After setting up both Diaper and Partner, log in Diaper, go to Partner Agencies
  • Add two Partner agencies and invite both of them
  • Log in the Partner app with one of the partners and request a review
  • In the Diaper app, there should be at least two Partners now: one invited and one awaiting review, both of which should have the "Review Application" action available
  • Click on the Review Application button of the "invited" partner. The Approve Partner button should be visible but disabled, and it should show a tooltip informing the user what to do.
  • Go back and click on the Review Application of the "awaiting review" partner. The Approve Partner button should be visible, enabled and should display no tooltip.

Automated testing:

For each of the partners above (one invited and one awaiting review) I added the following tests:

  • Test if the Approve Partner button is always visible
  • Test if the button is enabled for the "awaiting review" partner, but disabled for the "invited" partner
  • Test if the button displays a tooltip for the "invited" partner, but shows no tooltip for the "awaiting review" partner

Screenshots

Before changes

The Approve Partner button was at the bottom of the page, only visible and enabled for "awaiting review" partners and had no tooltip.

The button was never visible when the user was at the top of the page.
before-top-of-the-page

In case the user scrolled down, viewing a "invited" partner, the button was not visible:
before-bottom-of-the-page-invisible

In case the user scrolled down, viewing an "awaiting review" partner, the button was visible:
before-bottom-of-the-page

After changes

The button is now at a fixed position at the bottom right of the screen. The button is always visible, but disabled for "invited" partners. It also displays a tooltip for "invited" partners.

Desktop view:
screenshot-desktop

When scrolling to the bottom of the page:
screenshot-scroll-bottom

Mobile view:
screenshot-mobile

Tooltip:
disabled-tooltip

@edwinthinks
Copy link
Collaborator

@rhian-cs thanks for the PR! I've got a few suggestions now that I see the snapshots.

  • Could we add a background to the container of the button? That way it doesn't seem like a floating button but rather a bar on the bottom /w a button on it?
  • Could we move the button the bottom left instead? I realize the users are already accustomed to it being on the bottom left and putting it on the bottom right might cause a bit of confusions.

Otherwise I think this is looking great!

@rhian-cs
Copy link
Contributor Author

Hello, all changes were successfully added.

Desktop view at the middle of the page

print-desktop

Desktop view at the bottom of the page (unfortunately covers "built with ❤" text)

print-desktop-bottom

Mobile view at the middle of the page

I took the liberty of centering the button, but if that is undesirable, please let me know.
print-mobile

Mobile view at the bottom of the page

print-mobile-bottom

</div>
</div>
<div id="approve-application-button">
<% tooltip_message = "Partner has not requested approval yet. Partners are able to request approval going into 'My Organization' and clicking 'Request Approval' button" %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<% tooltip_message = "Partner has not requested approval yet. Partners are able to request approval going into 'My Organization' and clicking 'Request Approval' button" %>
<% tooltip_message = "Partner has not requested approval yet. Partners are able to request approval by going into 'My Organization' and clicking 'Request Approval' button." %>

Copy link
Collaborator

@edwinthinks edwinthinks left a comment

Choose a reason for hiding this comment

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

This looks good and I really like the specs you've added.

However, I would hold off until we deprecate the API calls as mentioned in one of my comments.

Comment on lines 235 to 245
def stub_get_partner_request(partner_id)
allow(ENV).to receive(:[]).and_return(ENV)
allow(ENV).to receive(:[]).with("PARTNER_REGISTER_URL").and_return("https://partner-register.com")
allow(ENV).to receive(:[]).with("PARTNER_BASE_URL").and_return("https://partner-register.com")
allow(ENV).to receive(:[]).with("PARTNER_KEY").and_return("partner-key")

partner_response = File.read("spec/fixtures/partner_api/partner.json")

stub_request(:get, "https://partner-register.com/#{partner_id}")
.to_return(status: 200, body: partner_response, headers: {})
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

This might be deprecated very soon as the update to main to merge the two applications has been merged in. That means, we won't need to make this API call once we've updated the controller to directly retrieve from the partner's database rather than go through the API.

I think we can hold off on this until that change which should be coming soon

@edwinthinks
Copy link
Collaborator

@rhian-cs hey there -- I've just merged in code to avoid needing to do the API calls in here. Could you update this code in accordance to the changes made in that PR? Thanks!

@rhian-cs
Copy link
Contributor Author

rhian-cs commented Mar 9, 2021

Hello there!
I've updated the specs.
The tests took some hours to finish on my machine, so I tested only the individual files when making the changes.

@edwinthinks
Copy link
Collaborator

@rhian-cs nice work! LGTM

The tests took some hours to finish on my machine, so I tested only the individual files when making the changes.

Yeh, it is a pretty large test suite. I personally just push up code and see if the CI can do it faster for me :)

@edwinthinks edwinthinks merged commit 3729b72 into rubyforgood:main Mar 9, 2021
@rhian-cs rhian-cs deleted the 2159-display-approve-partner-button-with-tooltip branch May 27, 2021 13:54
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.

Display the Approve Partner button on the top with a tooltip
2 participants