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: new form builder settings UI #7098

Merged
merged 88 commits into from
Jan 9, 2024

Conversation

pauloiankoski
Copy link
Contributor

@pauloiankoski pauloiankoski commented Nov 13, 2023

Resolves GIVE-21

Description

This pull request introduces a new interface for the Donation Form settings in the Visual Form Builder. Instead of the button to switch between "Edit Form" and "Edit Design" modes, there is now a tabs navigation. The settings have been moved from a sidebar in the "Edit Form" view to their own dedicated view. This new view includes a sidebar with a menu on the left side, and the settings are displayed in the main content area. Previously, certain settings were accessed through modals and popovers, but now they are directly available on the page for editing, such as email template options and donation confirmation editors.

It's important to note that no functionality has been changed from the previous version. This work primarily affects the organization of components, so everything continues to function as it did before.

Affects

Visual Form Builder settings UI only.

Visuals

CleanShot.2023-11-22.at.17.58.22.mp4

Testing Instructions

  1. Open the visual form builder
  2. Check that you can edit all settings
  3. Check that everything done has the same behavior that if done before this PR

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 feature/form-builder-settings-ui branch from a5ae78d to 27570f0 Compare November 14, 2023 00:50
@pauloiankoski pauloiankoski marked this pull request as ready for review November 22, 2023 21:08
@JasonTheAdams
Copy link
Contributor

Monumental effort, @pauloiankoski! I watched the video and I love it! Couple quick thoughts from the video:

  1. How does the settings UI flow responsively?
  2. I noticed that clicking on a menu with sub-menu items doesn't change the page. You click to see the sub-menu (or parent menu) and then click on an available menu. This is nice in the sense that you don't lose your spot, but it also feels odd to not have anything change, remaining in the current setting. I would think clicking "Email Settings" for example, would also pull up the "General" settings and save an extra click. It almost feels like something didn't work. (I'm curious what you think, too, @jdghinson.)

cc: @angelablake

@pauloiankoski
Copy link
Contributor Author

Thank you very much @JasonTheAdams!!!

  1. We didn't design how it would look on smaller screens. However, I've made some adjustments, such as moving the section header above the section content. This solves the issue to a great extent. Additionally, I had to hide the form title field from the top bar due to limited space for all the elements, especially in build mode and when the new undo/redo and embed form features are fully implemented. It's important to note that our Visual Form Builder is completely broken for mobile screens. Currently, it looks as follows:

CleanShot 2023-11-23 at 11 15 51

  1. Yes, my idea is based on how the reactified WordPress sidebar behaves in the Full-Site Editing mode. I'm open to changing it if you think it would be better to always have the first item opened when we switch between menu levels.

@jdghinson
Copy link
Contributor

Monumental effort, @pauloiankoski! I watched the video and I love it! Couple quick thoughts from the video:

  1. How does the settings UI flow responsively?
  2. I noticed that clicking on a menu with sub-menu items doesn't change the page. You click to see the sub-menu (or parent menu) and then click on an available menu. This is nice in the sense that you don't lose your spot, but it also feels odd to not have anything change, remaining in the current setting. I would think clicking "Email Settings" for example, would also pull up the "General" settings and save an extra click. It almost feels like something didn't work. (I'm curious what you think, too, @jdghinson.)

cc: @angelablake

I understand your concern and yeah it saves an extra click however it's a dropdown menu. Ideally with dropdown menus, users expect it to show the sub-menu not select the first item in the sub-menu. We need to match users mental model of this UI pattern.

@jdghinson
Copy link
Contributor

jdghinson commented Nov 23, 2023

Thank you very much @JasonTheAdams!!!

  1. We didn't design how it would look on smaller screens. However, I've made some adjustments, such as moving the section header above the section content. This solves the issue to a great extent. Additionally, I had to hide the form title field from the top bar due to limited space for all the elements, especially in build mode and when the new undo/redo and embed form features are fully implemented. It's important to note that our Visual Form Builder is completely broken for mobile screens. Currently, it looks as follows:

CleanShot 2023-11-23 at 11 15 51

  1. Yes, my idea is based on how the reactified WordPress sidebar behaves in the Full-Site Editing mode. I'm open to changing it if you think it would be better to always have the first item opened when we switch between menu levels.

I usually design for every screen size however for our form builder I don't see users using mobile to build their forms(I might be wrong). That explains why I usually go for desktop screen sizes. @pauloiankoski I love the way you changed the layout to fit the screen size. @JasonTheAdams @angelablake In case we might need to design for smaller screen sizes I can also do that, my concern/question is how many users build their forms using mobile?

Copy link
Contributor

@jdghinson jdghinson 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!

@jonwaldstein
Copy link
Contributor

@pauloiankoski i'm now getting this error when clicking the "view template tags" button in the email template settings:

Screenshot 2024-01-09 at 11 13 54 AM

@pauloiankoski
Copy link
Contributor Author

@jonwaldstein Resolved the scrolling issue on the TemplateTags here: 51e2dba

Copy link
Contributor

@jonwaldstein jonwaldstein left a comment

Choose a reason for hiding this comment

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

We made it! 🏁 Excellent work @pauloiankoski!!!

Let's please make sure to test every form setting in QA 🌈

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

Copy link
Contributor

@JasonTheAdams JasonTheAdams left a comment

Choose a reason for hiding this comment

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

Great work, @pauloiankoski! I think we're good to merge!

@pauloiankoski pauloiankoski merged commit 0f9447d into develop Jan 9, 2024
20 checks passed
@pauloiankoski pauloiankoski deleted the feature/form-builder-settings-ui branch January 9, 2024 23:47
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.

6 participants