Overview

The messages used in \Drupal\experience_builder\Controller\ErrorCodesEnum::getMessage are not helpful. It's unclear what the error is, and what the user could do differently to avoid the error.

Proposed resolution

User interface changes

Issue fork experience_builder-3520067

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

lauriii created an issue. See original summary.

meghasharma made their first commit to this issue’s fork.

meghasharma’s picture

meghasharma commented
Assigned: Unassigned » meghasharma

Working on it

meghasharma opened merge request !927

meghasharma’s picture

meghasharma commented
Assigned: meghasharma » Unassigned
Status: Active » Needs review

I've updated the messages in \Drupal\experience_builder\Controller\ErrorCodesEnum::getMessage() to make them more helpful and actionable.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Component: Page builder » Auto-save
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Messages in \Drupal\experience_builder\Controller\ErrorCodesEnum are not helpful » [Needs design] Messages in \Drupal\experience_builder\Controller\ErrorCodesEnum are not helpful
Assigned: Unassigned » tedbow
Status: Needs review » Needs work
Issue tags: +Needs design, +Needs screenshots

@tedbow: can you propose improved UI strings? 🙏

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » Unassigned

@lauriii since you made this issue did you run into these messages while using XB or just reviewing?

If you ran into it using XB what circumstances where they under? Locally where there was only one person using XB or where there might have been more?

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

I ran into this by making a change and then quickly trying to publish it after making that change. I think the error I got was "This item is unexpected." and it was really confusing because I had no idea what is "this" in the context of the message and why it was unexpected.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented

@lauriii thanks. Yeah that is the case that I could reproduce manually but not all the time.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Related issues: +#3520592: [PP-1] Do not allow a user Publish request while there are pending changes to the current layout

Created #3520592: [PP-1] Do not allow a user Publish request while there are pending changes to the current layout as idea how we might make this situation in #9 less likely

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [Needs design] Messages in \Drupal\experience_builder\Controller\ErrorCodesEnum are not helpful » [Needs design] Confusing messages when publishing immediately after making changes: auto-save race condition
Issue tags: +race condition, +Needs issue summary update

Clarifying issue title; issue summary could still use an update. Per @tedbow, #3520592: [PP-1] Do not allow a user Publish request while there are pending changes to the current layout probably is the right thing to do here, because that reduces the probability of ever seeing these messages.

meghasharma’s picture

meghasharma commented
Status: Needs work » Needs review

Updated the error messages in ErrorCodesEnum::getMessage() as per feedback from @tedbow.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » tedbow

Per #13.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Needs review » Reviewed & tested by the community

think is good. Thanks @meghasharma. Hopefully e2e test will pass otherwise because it is component-operations.cy.js is probably random fail

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Reviewed & tested by the community » Needs review

This is still marked as "Needs design" but I think it would be ok to merge the existing MR and then improve. The updated message would at least give the user something they can do that I think should resolve the problem. We can then improve

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

This definitely seems like an improvement over what was there before 👍

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » Unassigned
Status: Needs review » Reviewed & tested by the community

Ok. Let's get this and then figure how we can make it better

  • tedbow committed 8b33e997 on 0.x authored by meghasharma 
    Issue #3520067: [Needs design] Confusing messages when publishing...
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Reviewed & tested by the community » Fixed

Going to mark as fixed as #3520592: [PP-1] Do not allow a user Publish request while there are pending changes to the current layout should make it better.

If we still want to improve the messaging further, lets just reopen this issue

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.