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

Epic: add donation amount level descriptions #7364

Merged
merged 18 commits into from
May 14, 2024

Conversation

JoshuaHungDinh
Copy link
Contributor

@JoshuaHungDinh JoshuaHungDinh commented Apr 24, 2024

Resolves: GIVE-577

Requires: Form Builder Library release

Zeplin Design

Description

This task involves enhancing the Donation Amount block by adding text descriptions to be associated with each donation level. The default state of descriptions is disabled, however a toggle is now available to enable descriptions. Once enabled text area input fields are shown for each level.

To accomplish this we have reduced all the previously split attributes for the level into a single levels attribute. The levels attribute now holds default value, the level value, the label(description) & an optional id.

Additional changes:

  • Updated UI for the Donation Options Inspector controls/settings
  • New UI for the block preview & frontend

Pull Requests:

Fix: Update Currency Switcher integration #7366

Refactor: Update DonationAmountLevels template to support descriptions #7363

Refactor: Change v2 to v3 migration to include level descriptions #7362

Refactor: Merge block level attributes #7361

Refactor: update amount node conversion with descriptions & descriptionsEnabled #7355

Feature: add level descriptions block-placeholder #7354

Feature: update amount block settings with descriptions #7353

Refactor: update price option settings control #7350

Affects

The Donation Amount block.

Visuals

donation.type.control.mov
description.formbuilder.mov
Frontend.UI.mov

Testing Instructions

Donation Type Control:

  • Switch new toggle between Multi & Fixed.
  • The block preview and inspector controls should toggle displaying different previews and settings.

Descriptions FormBuilder Preview/Controls:

  • Toggle the Enable descriptions setting, verify the description controls and the new block preview in the editor display.
  • Try updating some of the descriptions, as well as selecting a new default & rearranging the order. You should see these changes live in both the controls and the editor preview.
  • Verify the descriptions setting publishs/saves properly.

Frontend UI:

  • Verify the new descriptions display on the frontend with the new design.

v2 -> v3 Migration:

  • Create a v2 form using donation levels.
  • Migrate that form to v3.
  • Confirm that levels have been migrated properly.

Pre-review Checklist

  • Acceptance criteria satisfied and marked in related issue
  • Relevant @unreleased tags included in DocBlocks
  • Includes unit tests
  • Reviewed by the designer (if follows a design)
  • Self Review of code and UX completed

@pauloiankoski pauloiankoski force-pushed the epic/donation-amount-level-descriptions branch from 5d1ddd8 to 0d1ec0c Compare April 24, 2024 01:38
@JoshuaHungDinh JoshuaHungDinh marked this pull request as ready for review April 24, 2024 19:38
Copy link
Member

@rickalday rickalday left a comment

Choose a reason for hiding this comment

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

Passed manual QA tests.

@pauloiankoski pauloiankoski self-assigned this Apr 29, 2024
@pauloiankoski pauloiankoski force-pushed the epic/donation-amount-level-descriptions branch from 48ba42b to a7c7761 Compare April 30, 2024 16:33
@pauloiankoski pauloiankoski force-pushed the epic/donation-amount-level-descriptions branch from e36ab4f to 6d97fd8 Compare April 30, 2024 22:54
@jonwaldstein jonwaldstein self-requested a review May 2, 2024 13:58
@jonwaldstein
Copy link
Contributor

@pauloiankoski @JoshuaHungDinh I tried to run build and i'm getting an error, do we need another form builder library release?

Screenshot 2024-05-02 at 11 14 24 AM

@pauloiankoski
Copy link
Contributor

@jonwaldstein Indeed. We have implemented the max length in the description field. Without updating the package, the PR might break.

@jonwaldstein
Copy link
Contributor

@pauloiankoski okay good to know, in the future we don't want to merge features that depend on external releases until it's safe to do so - in this case we needed the form builder library release before we can merge the implementation PR. This would be an example of a blocker that you can raise to me to help unblock.

@jonwaldstein
Copy link
Contributor

@pauloiankoski @JoshuaHungDinh @rickalday

I'd like to add an additional QA test to confirm the donation level schema migration for backwards compatibility.

  1. Before using this PR, create a v3 form with donation amount levels
  2. Switch to this PR and confirm the existing donation amount levels are working properly

We need to confirm existing donation amount blocks are migrated to the new schema successfully. This is very important to make sure or else it could significantly impact existing donation amount fields.

Side note @pauloiankoski I believe we can write a unit test for the run() method in UpdateDonationLevelsSchema to assert this as well 😄

I ran into this issue that could very well be a js cache'ing problem on my end - but it made me want to circle back and confirm the migration.

Screenshot 2024-05-02 at 11 50 58 AM

@pauloiankoski pauloiankoski force-pushed the epic/donation-amount-level-descriptions branch from 4a2c6a5 to ca0b514 Compare May 2, 2024 21:38
@pauloiankoski
Copy link
Contributor

Side note @pauloiankoski I believe we can write a unit test for the run() method in UpdateDonationLevelsSchema to assert this as well 😄

@jonwaldstein Test implemented on ca0b514

@jonwaldstein
Copy link
Contributor

@JoshuaHungDinh @pauloiankoski I updated the form builder library to new version 0.6.0

@jonwaldstein jonwaldstein merged commit 4293f07 into develop May 14, 2024
32 checks passed
@jonwaldstein jonwaldstein deleted the epic/donation-amount-level-descriptions branch May 14, 2024 14:27
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.

4 participants