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

issue #4979: Donation site screen improvement #4991

Merged
merged 6 commits into from
Feb 7, 2025

Conversation

Oce-ane
Copy link
Contributor

@Oce-ane Oce-ane commented Feb 3, 2025

Resolves #4979

Description

This PR updates the donation site screen to align with the rest of the system by:

  • Removing the redundant row of fields and create button at the top of the page.
  • Keeping only the "New Donation Site" button for consistency with other pages.
  • Updating documentation (docs/user_guide/bank/community_donation_sites.md) to reflect the new page layout.
  • Deleting an unused image that is no longer relevant.
  • Deleting the now irrelevant tests

Type of change

  • Removed the row of fields and the create button from the Donation Sites index view.
  • Deleted the associated partial form (since it is no longer used).
  • No test modifications were necessary, as no specs referenced the removed fields.
  • Documentation update
    *Rspec update

How Has This Been Tested?

  • Manually verified that the "New Donation Site" button remains functional.
  • Checked that the index page now matches the behavior of other similar pages.
  • Reviewed updated documentation for accuracy.
  • Deleted the relevant tests for the removed fields.

Screenshots

4979_screenshot

cielf
cielf previously requested changes Feb 3, 2025
Copy link
Collaborator

@cielf cielf left a comment

Choose a reason for hiding this comment

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

Functionality: works well!

I'm still seeing an image in the guide that's showing the old screen, though.

Further comment:
I'm not the main tech reviewer (@dorner is) but I'm wondering if the reference to table_row_prepend that's only in donation_sites_controller, should also be adjusted (and the partial removed) for consistency with the other community pages.


## The Donation Site list
You can manage the sites' information on the "Donation Sites" page under the "Community" section.
You can manage the sites' information on the "Donation Sites" page under the "Community" section.

![Donation Sites](images/community/donation_sites/donation_sites.jpg)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This image is still showing the old screen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have made the change. Should I wait for confirmation or go ahead with the change on table_row_prepend part ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm about 95% sure that that change should be made. So please go ahead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Oce-ane My apologies, I have a tendency to stop when I see a problem -- there are two more images shown in community_donation_sites.md that are showing the old screen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, really sorry about that ! I think I got them all this time

@cielf cielf requested a review from dorner February 4, 2025 21:22
@Oce-ane
Copy link
Contributor Author

Oce-ane commented Feb 5, 2025

Description

The last two commits (1a29023 & 7356c7e) remove the unused table_row_prepend partial, which was linked to a previous form that was removed in an earlier commit addressing this issue.

Changes made:

  • Removed the table_row_prepend partial after verifying it was no longer referenced in the codebase.
  • Manually tested by creating a new donation site and confirming no network requests or logs related to the partial.
  • Fixed a small typo in the documentation introduced in previous commits.

Type of change

  • Code cleanup (removing unused files)

How Has This Been Tested?

  • Checked logs & network requests: Confirmed that the partial was no longer being rendered.
  • Manual testing: Created a new donation site and verified that the UI behaved correctly without the partial.
  • Reviewed documentation: Fixed a typo in docs/user_guide/bank/community_donation_sites.md.

@cielf
Copy link
Collaborator

cielf commented Feb 6, 2025

This is ready for a quick technical review, @dorner. (Needs a guide update, but that shouldn't impact the tech review.)

@cielf cielf dismissed their stale review February 7, 2025 00:09

Addressed. LGTM

Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

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

Looks good on tech side.

@cielf cielf merged commit 69c4891 into rubyforgood:main Feb 7, 2025
12 checks passed
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.

Donation Site Screen improvement
3 participants