Skip to content

[Page theme] Formscape #14527

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 18 commits into from
Feb 24, 2025
Merged

[Page theme] Formscape #14527

merged 18 commits into from
Feb 24, 2025

Conversation

andersonjeccel
Copy link
Contributor

@andersonjeccel andersonjeccel commented Jan 29, 2025 •
edited
Loading

Q A
Bug fix? (use the a.b branch) 🟢
New feature/enhancement? (use the a.x branch) 🟢
Deprecations? 🔴
BC breaks? (use the c.x branch) 🔴
Automated tests included? 🔴
Related user documentation PR URL mautic/user-documentation#...
Related developer documentation PR URL mautic/developer-documentation-new#...
Issue(s) addressed Fixes #...

Description

Adds a page theme: half a column image, and 30% for a form

✅ Responsive with no glitches
✅ Mobile first
✅ What you see in the builder is what you get
✅ 100% compatible with GrapesJS
✅ Really, really fast

Warning

Git configuration added as required by project lead
This PR depends on a fix released in 5.2 that wasn't merged onto 6.x yet

Desktop

Screenshot_29-1-2025_134816_127 0 0 1

Mobile

127 0 0 1_32787_page_preview_12

📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Open Landing pages > New
  3. Pick Formscape
  4. Open the builder and have fun

@andersonjeccel andersonjeccel requested review from a team, LordRembo and laurielim January 29, 2025 16:58
@andersonjeccel andersonjeccel self-assigned this Jan 29, 2025
@andersonjeccel andersonjeccel added T1 Low difficulty to fix (issue) or test (PR) ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging enhancement Any improvement to an existing feature or functionality forms Anything related to forms landing-pages Anything related to landing pages themes Anything related to themes builder-grapesjs Anything related to the GrapesJS email or landing page builders labels Jan 29, 2025
@andersonjeccel andersonjeccel added this to the 6.0.0-beta milestone Jan 29, 2025
@codecov Codecov
Copy link

codecov bot commented Jan 29, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.24%. Comparing base (02080c2) to head (3eea9f1).
Report is 2 commits behind head on 6.x.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                6.x   #14527      +/-   ##
============================================
- Coverage     64.24%   64.24%   -0.01%     
  Complexity    34576    34576              
============================================
  Files          2268     2268              
  Lines        103294   103294              
============================================
- Hits          66366    66365       -1     
- Misses        36928    36929       +1

see 1 file with indirect coverage changes

escopecz
escopecz reviewed Jan 30, 2025
View reviewed changes
Copy link
Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

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

The PR description should mention that this is also refactoring the email themes and those should be tested too.

@andersonjeccel andersonjeccel added the user-experience Anything related to related to workflows, feedback, and navigation label Jan 30, 2025
matbcvo
matbcvo reviewed Feb 7, 2025
View reviewed changes
@andersonjeccel andersonjeccel mentioned this pull request Feb 21, 2025
Merged
RCheesley
RCheesley requested changes Feb 21, 2025
View reviewed changes
Copy link
Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

The reachout theme's GitHub Workflow file is erroneously added here, I noticed it was brought up in another review. It should be removed from this PR and added in the reachout PR please. Thanks!

@RCheesley RCheesley added the pending-feedback PR's and issues that are awaiting feedback from the author label Feb 21, 2025
@ricfreire
Copy link

I tested the theme as requested by @RCheesley.

image
The .sidebars class is currently set to align-items: center;, but it should be changed to align-items: flex-start; to prevent the glitch occurring on mobile.

image
The .sidebar--right class has a min-width of 290px, which is unnecessary and causes the image to exceed the screen size on very small devices. Since it already has width: 100%;, responsiveness is ensured without the need for a min-width constraint.

  1. (Optional)
    image
    Adjusting the spacing for better usability:
  • Set margin-bottom: 20px; on .subtitle.header
  • Set margin-top: 10px; on .subtitle.footer
    This will reduce the need for scrolling when the form contains 2 or 3 fields, making the entire form more accessible without excessive scrolling. The fix maintains a clean look on both mobile and desktop views.

@andersonjeccel andersonjeccel added pending-test-confirmation PR's that require one test before they can be merged code-review-passed PRs which have passed code review and removed ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging pending-feedback PR's and issues that are awaiting feedback from the author labels Feb 21, 2025
@matbcvo matbcvo added the has-conflicts Pull requests that cannot be merged until conflicts have been resolved label Feb 21, 2025
RCheesley
RCheesley requested changes Feb 21, 2025
View reviewed changes
Copy link
Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

Needs a fix to the minimum version here.

@matbcvo matbcvo removed the has-conflicts Pull requests that cannot be merged until conflicts have been resolved label Feb 24, 2025
@RCheesley RCheesley merged commit 69fc221 into mautic:6.x Feb 24, 2025
18 checks passed
@andersonjeccel andersonjeccel deleted the formscape-theme branch February 24, 2025 14:07
@RCheesley RCheesley modified the milestones: 6.0.0-beta, 6.0.0-beta2 Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matbcvo matbcvo matbcvo left review comments

@escopecz escopecz escopecz left review comments

@RCheesley RCheesley RCheesley requested changes

@LordRembo LordRembo Awaiting requested review from LordRembo LordRembo was automatically assigned from mautic/ux-ui-tiger-team

@laurielim laurielim Awaiting requested review from laurielim laurielim was automatically assigned from mautic/ux-ui-tiger-team

Assignees

@andersonjeccel andersonjeccel

Labels
builder-grapesjs Anything related to the GrapesJS email or landing page builders code-review-passed PRs which have passed code review enhancement Any improvement to an existing feature or functionality forms Anything related to forms landing-pages Anything related to landing pages pending-test-confirmation PR's that require one test before they can be merged T1 Low difficulty to fix (issue) or test (PR) themes Anything related to themes user-experience Anything related to related to workflows, feedback, and navigation
Projects
Open Source Fridays
Status: 🥳 Done
UX/UI Tiger Team
Status: Done
Milestone
6.0.0-beta2
Development

Successfully merging this pull request may close these issues.
6 participants
@andersonjeccel @ricfreire @matbcvo @escopecz @RCheesley @driskell