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

[UX] Improve saving options #13713

Draft
wants to merge 26 commits into
base: 5.x
Choose a base branch
from

Conversation

andersonjeccel
Copy link
Contributor

@andersonjeccel andersonjeccel commented May 1, 2024

Q A
Bug fix? (use the a.b branch) [ ]
New feature/enhancement? (use the a.x branch) [x]
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:

Modern apps tend to use only "save/cancel" to simplify the user experience and keep the interface clean and intuitive. This PR ensures that the system performs the appropriate action in the user context.

When clicking Save, the system will automatically close the editing screen in these scenarios:

  • Contacts
  • Companies
  • Forms
  • Email
  • Dynamic Content
  • Campaigns
  • Categories
  • Tags
  • Reports
  • Stages
  • Points (Actions, Triggers and Groups)
  • Segments
  • Landing pages
  • Assets
  • Roles
  • Users
  • Focus Items
  • Mobile Notifications
  • Web Notifications
  • API
  • SMS
  • Custom Fields
  • Plugins

In these other scenarios, the system will simply apply the changes without taking the user out of the page:

  • Configuration
  • Account

Why must we change that?

  1. UX research indicates that many options can overwhelm users, leading to indecision or even inaction, a phenomenon known as analysis paralysis.
  2. An excessive number of choices increases cognitive load, making the decision-making process more complex and tiresome for the user.
  3. Reducing options can enhance efficiency, allowing users to complete their tasks more quickly and with less effort.
  4. Studies show that, while users may initially appreciate having many options, they often feel more satisfied when the decision-making process is straightforward and direct.
  5. Simpler interfaces are easier to learn and remember, which is especially important for new users or those who use the system sporadically.
  6. Maintaining consistency with recognized design patterns helps users anticipate system behavior, which can be disrupted by the introduction of additional options.

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 Tags, for example, > New and see you have only Save and Cancel

@andersonjeccel andersonjeccel self-assigned this May 1, 2024
@andersonjeccel andersonjeccel added T1 Low difficulty to fix (issue) or test (PR) user-experience Anything related to related to workflows, feedback, and navigation enhancement Any improvement to an existing feature or functionality php Pull requests that update Php code labels May 1, 2024
@andersonjeccel
Copy link
Contributor Author

andersonjeccel commented May 1, 2024

PHPUnit tests are probably failing because of this line:

$saveButton = $crawler->selectButton('Save');

Every button is still called Save, but something on the code associates it with Apply instead of Save & Close, and since I set the Apply button as false, the tests can't find the button

@LordRembo or @mallezie Could you help me? I need to know what must change for tests to properly use Save & Close

@mallezie
Copy link
Contributor

mallezie commented May 2, 2024

problem is that there is changed behavior, for which tests need to be adjusted.
Example:
Before patch: Add new api credential credentials/new
When you fill in values, and click save, this keeps you on the created api key page, wich is wat the Oauth2Test class expects.
After patch:
Fill in values, click save and go to the overview, the oauth2test class expects still to be on the edit api key page, and hence fails following test checks (which checks form fields on that page).

Current Oauth2Test does following.

    // Create OAuth2 credentials.
    // Fill in the form with the values. 
    $crawler    = $this->client->request(Request::METHOD_GET, 's/credentials/new');
    $saveButton = $crawler->selectButton('Save');
    $form       = $saveButton->form();
    $form['client[name]']->setValue('Auth Test');
    $form['client[redirectUris]']->setValue('https://test.org');

     // Submit the form
    $crawler = $this->client->submit($form);
    // Check submit form succeeded.
    Assert::assertTrue($this->client->getResponse()->isOk(), $this->client->getResponse()->getContent());
    
   // From below failures occur with patch. Below code assumes we're now on the edit credentials form, instead of now we're on the api overview page. 
    $clientPublicKey = $crawler->filter('input#client_publicId')->attr('value');
    $clientSecretKey = $crawler->filter('input#client_secret')->attr('value');

To change above, before checking the values of the form, we need to go back to the created api credentials form. Actually go to s/credentials/edit/

Herefore we could (untested) change code something like this.

    Assert::assertTrue($this->client->getResponse()->isOk(), $this->client->getResponse()->getContent());

    $clientPublicKey = $crawler->filter('input#client_publicId')->attr('value');
    $clientSecretKey = $crawler->filter('input#client_secret')->attr('value');

Should become something like this.

    $response = $this->client->getResponse();
    Assert::assertTrue($response->isOk(), $response->getContent());

     // In the response we somewhere need to find the id of the created credentials if possible, else just go back to the one we created, we know it's the first one created. 
    $crawler    = $this->client->request(Request::METHOD_GET, 's/credentials/edit/1');

    $clientPublicKey = $crawler->filter('input#client_publicId')->attr('value');
    $clientSecretKey = $crawler->filter('input#client_secret')->attr('value');

@andersonjeccel andersonjeccel marked this pull request as draft May 14, 2024 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Any improvement to an existing feature or functionality php Pull requests that update Php code T1 Low difficulty to fix (issue) or test (PR) user-experience Anything related to related to workflows, feedback, and navigation
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

None yet

2 participants