Skip to content

Conversation

@andersonjeccel
Copy link
Contributor

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/mautic-documentation#...
Related developer documentation PR URL mautic/developer-documentation#...
Issue(s) addressed Fixes #...

Description:

This update enriches the redirection options during landing page creation with descriptive labels. It aims to guide users in selecting the appropriate redirect type by explaining the difference between temporary and permanent redirects, and when each should be used. This clarification assists users in making informed decisions, ensuring their landing pages function as intended and enhancing the overall creation process.

Now people can just select the less complicated option to fit 99,99% of their needs.

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
  3. Create a new
  4. Click to see the options for Redirection

image

@andersonjeccel andersonjeccel added ready-to-test PR's that are ready to test user-experience Anything related to related to workflows, feedback, and navigation enhancement Any improvement to an existing feature or functionality labels Feb 28, 2024
@andersonjeccel andersonjeccel self-assigned this Feb 28, 2024
@andersonjeccel andersonjeccel added this to the 5.0.4 milestone Feb 28, 2024
@codecov
Copy link

codecov bot commented Feb 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.59%. Comparing base (6137414) to head (d883d82).
Report is 49 commits behind head on 5.x.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                5.x   #13490      +/-   ##
============================================
+ Coverage     60.57%   60.59%   +0.01%     
- Complexity    33835    33845      +10     
============================================
  Files          2231     2232       +1     
  Lines        101135   101179      +44     
============================================
+ Hits          61260    61305      +45     
+ Misses        39875    39874       -1     

see 2 files with indirect coverage changes

@andersonjeccel andersonjeccel added the landing-pages Anything related to landing pages label Feb 28, 2024
@LordRembo
Copy link
Contributor

I think this is missing a bit more info. If a user doesn't know (or look up) the redirection code, it's probably not going to be solved by adding an extra bit like (GET) to it. A user that has to make this choice, has to be willing to read what the actual difference is between the error codes.
Some ways to make it easier for them, is to provide descriptions (but where would you put that?) or a link to documentation explaining the different codes (eg. link in the tooltip to MDN's page)

@andersonjeccel
Copy link
Contributor Author

@LordRembo
It should make a little bit easier for people to select a simple item that helps them to do what needs to be done, but yeah, it's really far from a perfect solution. 😅

I'm struggling to change the form structure (adding a link below the field, for example). Do you have a tip about where should I look?

Then may I create a separate PR or change this one?

@LordRembo
Copy link
Contributor

@andersonjeccel I'm also not sure how to extend the form. I can see it's built partly in app/bundles/PageBundle/Form/Type/PageType.php and app/bundles/PageBundle/Form/Type/RedirectListType.php, with a generic form view but no idea how it all works.
A separate ticket would be good, then we can check & link to get some backend help.

@andersonjeccel
Copy link
Contributor Author

@LordRembo Hm, thanks for checking!

I'll give it a try next week

Copy link

@Esthertests Esthertests left a comment

Choose a reason for hiding this comment

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

LGTM
on prod
image

On pr
image

@RCheesley RCheesley added T1 Low difficulty to fix (issue) or test (PR) code-review-needed PR's that require a code review before merging pending-test-confirmation PR's that require one test before they can be merged needs-documentation PR's that need documentation before they can be merged and removed ready-to-test PR's that are ready to test labels Mar 15, 2024
@andersonjeccel
Copy link
Contributor Author

@RCheesley Where do we need to change documentation?

I think we can merge this pull request. We can create another later focusing on explaining the different redirection types (but anyone can search on the internet what each means in the meantime).

@andersonjeccel andersonjeccel added user-testing-passed PRs which have been successfully tested by the required number of people. and removed pending-test-confirmation PR's that require one test before they can be merged labels Mar 20, 2024
@escopecz escopecz modified the milestones: 5.0.4, 5.1.0 Apr 2, 2024
Copy link
Member

@kuzmany kuzmany left a comment

Choose a reason for hiding this comment

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

Basic users will not understand with or without change. But go

@andersonjeccel
Copy link
Contributor Author

@kuzmany

I started a discussion on Slack to suggest how to solve this:
https://mautic.slack.com/archives/CQGQ0D4KU/p1710331987374019

but it'll be a separate PR

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.

Hi folks!

I think this is confusing for the user - (GET) and (HTTP) is just using developer language which we're professing to be moving away from.

I agree that as a better approach, we should write up an article on the Knowledgebase which explains the redirect codes. It would be a useful article in general. For example, from my brief reading a 308 is being favoured for SEO reasons nowadays as being a 'cleaner' redirect.

So I'm not in favour of this PR being merged, instead we should help users understand what the error codes mean and why you might use them.

@RCheesley RCheesley closed this Apr 4, 2024
@andersonjeccel andersonjeccel deleted the ux-specify-redirection-labels-present-when-creating-a-landing-page branch May 1, 2024 15:22
@andersonjeccel andersonjeccel restored the ux-specify-redirection-labels-present-when-creating-a-landing-page branch May 1, 2024 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-review-needed PR's that require a code review before merging enhancement Any improvement to an existing feature or functionality landing-pages Anything related to landing pages needs-documentation PR's that need documentation before they can be merged T1 Low difficulty to fix (issue) or test (PR) user-experience Anything related to related to workflows, feedback, and navigation user-testing-passed PRs which have been successfully tested by the required number of people.

Projects

Archived in project
Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants