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

Feature: add excerpt for v3 forms #7287

Merged
merged 7 commits into from
Mar 19, 2024

Conversation

glaubersilva
Copy link
Contributor

@glaubersilva glaubersilva commented Mar 11, 2024

Resolves GIVE-165

Description

This PR added a new "Excerpt" text area option in the V3 general form settings to be used to add a summary to the forms. This way, we can fix a problem in the Form Grid shortcode/block/widget that has this value missing for V3 forms due to the missing of this option. Also, this PR adds a migration step to transfer the "Excerpt" option from V2 to V3 forms.

Affects

  • The V3 form general settings
  • The form grid shortcode/block/widget
  • The migration form process

Visuals

https://www.loom.com/share/25692c76f0ce4e04bf55c12bb0edc19c?sid=21d46133-64bd-4d9a-a9c4-09f4a5b6df72

image

Testing Instructions

  1. Create or edit a V3 form and fill the new Excerpt text area in the general settings;
  2. Add the Form Grid widget to an Elementor page;
  3. Set the "Show Excerpt" to "true";
  4. The "Excerpt" now should be visible in the form grid.

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

@glaubersilva glaubersilva self-assigned this Mar 13, 2024
@glaubersilva glaubersilva marked this pull request as ready for review March 13, 2024 17:22
@glaubersilva glaubersilva requested a review from alaca March 13, 2024 17:22
Copy link
Member

@alaca alaca left a comment

Choose a reason for hiding this comment

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

Adding stuff to the core to support other plugins is not something that we want to do. If we have to add new features/options to extend the core functionality, we should always do that from the add-ons.

We already have the form description as a property of the DonationForm object (DonationForm->settings->description), so why don't we use that? Also, I know that the excerpt option exists for the v2 forms, but correct me if I'm wrong, we don't display that anywhere on the form?

Anyway, I suggest closing this PR and just using the description from the v3 form settings as a workaround.

@glaubersilva
Copy link
Contributor Author

glaubersilva commented Mar 14, 2024

@alaca We are not adding stuff in the core to support other plugins. The give_form_grid_shortcode( $atts ) lives in the Core and relies on the Form Excerpt options missing on V3 forms.

Are you suggesting using the "Description" from the attached screenshot right? If yes, I don't think it can be used as a workaround because it's just a description of the first section of the form and not the whole form.

For example, I can change this description to something like "Any value makes the difference!" and it will not be a good fit to use as a form summary such as the form excerpt would be.

image

@glaubersilva glaubersilva requested a review from alaca March 14, 2024 13:31
@alaca
Copy link
Member

alaca commented Mar 14, 2024

Hey @glaubersilva, sorry for the confusion. I was talking about the header settings.

Screen Capture_select-area_20240314145339

I think this is exactly what you need here.

We are not adding stuff in the core to support other plugins.

Okay, but then why do I need Elementor to test it 😅

I understand that this excerpt option exists in give_form_grid_shortcode, but do we really need that option for v3 forms? I'm not convinced that we do. Especially if that option is something that very few people use. We already have something similar (if not the same) we can use here.

@glaubersilva
Copy link
Contributor Author

glaubersilva commented Mar 14, 2024

@alaca Thanks for the clarification.

About this:

Okay, but then why do I need Elementor to test it

I suggested testing with Elementor only because the problem was originally reported in the Elementor widget which uses the shortcode under the hood, but it can be tested directly with the shortcode or even the Gutenberg block.

About using the header description instead of the form excerpt, I believe it can cause some misunderstanding because the shortcode attribute is 'show_excerpt' and not 'show_header_description', so I'm still not convinced that we can do this replacement without generating some kind of confusion to our users.

I understand your point when you say that these options are very similar. However, the same thing I said before for the section description can be applied here in the header description because this text also can not be a good fit to be used as a form summary such as the form excerpt would be.

For example, I can set a generic text on it like "Donate today!" and it will not tell me too much about why I should donate as the excerpt option proposes to do. Or, I can simply remove this header description due to a design choice, and this way the "excerpt" also would be removed if we choose to use the header description as a replacement for the excerpt.

But yeah, we could add a notice below the header description option saying that it would be used as the form excerpt as well and force users to keep it filled with more descriptive text if they want to use the excerpt. From a UX perspective, I don't think this is a good solution.

This way, I would like to know the perspective of @jdghinson about it before we take any decision.

@alaca
Copy link
Member

alaca commented Mar 14, 2024

I suggested testing with Elementor only because the problem was originally reported in the Elementor widget which uses the shortcode under the hood, but it can be tested directly with the shortcode or even the Gutenberg block.

Sorry, it was a joke. I was trying to be funny here, but it didn't work as expected.

I understand your point when you say that these options are very similar. However, the same thing I said before for the section description can be applied here in the header description because this text also can not be a good fit to be used as a form summary such as the form excerpt would be.

For example, I can set a generic text on it like "Donate today!" and it will not tell me too much about why I should donate as the excerpt option proposes to do. Or, I can simply remove this header description due to a design choice, and this way the "excerpt" also would be removed if we choose to use the header description as a replacement for the excerpt.

Let's see what we have now on the edit v2 form screen:

Excerpts are optional hand-crafted summaries of your content that can be used in your theme.

What does that tell me? Not much. I can put whatever here, even "Donate today!". But the header description on the other hand, basically sums up what the donation form is used for.

If the site admin hides the header description, that doesn't mean it's erased, that information is still there and we can use it. It won't show up on the form, but we have it.

Anyway, I'm still not convinced we need this option, especially because many people don't even know or use this option. To be honest, I didn't even know what that option on the edit v2 form screen is used for. And also the fact that we already have a pretty good alternative we can use, I'm even more convinced that we don't need it.

@alaca
Copy link
Member

alaca commented Mar 14, 2024

@jonwaldstein any thoughts?

@glaubersilva
Copy link
Contributor Author

glaubersilva commented Mar 14, 2024

@alaca My feedback about your last comment:

Sorry, it was a joke. I was trying to be funny here, but it didn't work as expected.

My bad! I did read that in a literal way and ignored the emoji. Sorry!

JustSmileSheldonCooperGIF

Excerpts are optional hand-crafted summaries of your content that can be used in your theme.

Yep! This is what the default helper text from the metabox added by WP says but is not the same as the "Form Excerpt" option in the Give default options page - check the attached screenshot for reference.

If the site admin hides the header description, that doesn't mean it's erased, that information is still there and we can use it. It won't show up on the form, but we have it.

This is a good point!

However, I still would like to know the perspectives of @jdghinson and @jonwaldstein before any decision.

image

@jdghinson
Copy link

@alaca My feedback about your last comment:

Sorry, it was a joke. I was trying to be funny here, but it didn't work as expected.

My bad! I did read that in a literal way and ignored the emoji. Sorry!

JustSmileSheldonCooperGIF JustSmileSheldonCooperGIF

Excerpts are optional hand-crafted summaries of your content that can be used in your theme.

Yep! This is what the default helper text from the metabox added by WP says but is not the same as the "Form Excerpt" option in the Give default options page - check the attached screenshot for reference.

If the site admin hides the header description, that doesn't mean it's erased, that information is still there and we can use it. It won't show up on the form, but we have it.

This is a good point!

However, I still would like to know the perspectives of @jdghinson and @jonwaldstein before any decision.

image

I have some few questions @glaubersilva

  1. Are we using the header description in V2 for this form description? If yes, then why have the excerpt option? Is there any use case I’m missing?
  2. Do we have numbers to support this claim of few people using the excerpt? @alaca Cos we still have a huge chunk of users using the v2 forms and if they are using that, we need to have it in v3 since we have plans on making the visual form builder our default.

@glaubersilva
Copy link
Contributor Author

@jdghinson Here are my replies to your questions:

  1. Nope! We don't use the header description in V2 forms. As I said before, we use the excerpt option in the show_excerpt attribute of the Form Grid. This is the reason I implemented the same feature in the form builder.
  2. I have no idea, but I believe this question was for @alaca .

@alaca
Copy link
Member

alaca commented Mar 15, 2024

Do we have numbers to support this claim of few people using the excerpt?

@jdghinson I don't, but we can ask the support team if anyone reported this option missing after migrating to v3.

@jdghinson
Copy link

Do we have numbers to support this claim of few people using the excerpt?

@jdghinson I don't, but we can ask the support team if anyone reported this option missing after migrating to v3.

We can do that however that wouldn’t give us a fair insight on the usage like I said we still have a lot of our users still using v2.

@rickalday do we have insights on the number of users using the form excerpt option?

@rickalday
Copy link
Member

@jdghinson we don't have numbers on that. I know some users rely on the excerpt when they use the form grid. There are some SEO plugins that also rely on the excerpt for OG description tags so that's something to consider.

@jdghinson
Copy link

@jdghinson we don't have numbers on that. I know some users rely on the excerpt when they use the form grid. There are some SEO plugins that also rely on the excerpt for OG description tags so that's something to consider.

@alaca We should have the excerpt in v3 since we don’t have enough information on this and SEO means a lot to marketing. To be on a safer side let’s maintain this feature.

Cc: @glaubersilva

Copy link
Member

@alaca alaca left a comment

Choose a reason for hiding this comment

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

Let's merge this PR. I'm still not convinced that we need it, but someone might find it useful, and probably some people are already using this option.

Codewise, great work @glaubersilva 👍

@jonwaldstein
Copy link
Contributor

Great discussion all! These conversations are really important as we look toward the future of GiveWP and I appreciate the healthy, professional debate 🦾. We are in new territory right now where we have to decide which features we want to carry into the future, so all points of views are welcome 🌈

In conclusion from my side:

  • We should try our best to preserve any existing data & functionality from v2 to v3 forms.
  • We want add-ons to use more core features of GiveWP instead of rolling their own - as they can become more of challenge to maintain.

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.

@glaubersilva glaubersilva merged commit b4923b9 into develop Mar 19, 2024
20 checks passed
@glaubersilva glaubersilva deleted the feature/v3-forms-excerpt-GIVE-165 branch March 19, 2024 15: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.

5 participants