Overview

Proposed resolution

Create an api slice to `getAllPendingChanges` that reads from the controller added in #3489743: Create AutoSave service and HTTP API to retrieve all entities with pending changes.

User interface changes


Review N Changes

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:

Comments

larowlan created an issue. See original summary.

larowlan’s picture

Status: Active » Postponed
lauriii’s picture

Title: [PP-1] Create api slice for auto-save data (pending changes) and publishing all data » Create api slice for auto-save data (pending changes) and publishing all data
Status: Postponed » Active
soaratul’s picture

Assigned: Unassigned » soaratul

soaratul’s picture

Assigned: soaratul » Unassigned
Status: Active » Needs review
hooroomoo’s picture

Left some comments

effulgentsia’s picture

Title: Create api slice for auto-save data (pending changes) and publishing all data » Implement the "Review N changes" button
Issue summary: View changes
Related issues: +#3497530: Implement the "Publish All" button

I updated this issue's title and description to reflect a smaller scope: just the "Review N changes" button and what it shows when you click it. I opened #3497530: Implement the "Publish All" button for the "Publish All" button.

effulgentsia’s picture

Assigned: Unassigned » soaratul
Status: Needs review » Needs work

It looks to me like there are still some unresolved comments on the MR, so setting to "Needs work".

soaratul’s picture

Assigned: soaratul » Unassigned
Status: Needs work » Needs review
larowlan’s picture

Issue summary: View changes
StatusFileSize
new52.97 KB

Added screenshot to issue summary

wim leers’s picture

Assigned: Unassigned » soaratul
Status: Needs review » Needs work
Issue tags: +Needs screenshots

I'd like to get a short screencast or a GIF in place to demonstrate this, because a static screenshot makes it hard to get a sense of the impact. 🙏

The (static) screenshots do look interesting already though! 😄

(Also a few high-level questions. Also: random failures on PostgreSQL CI runs, re-queued those 👍)

soaratul’s picture

Assigned: soaratul » Unassigned
Status: Needs work » Needs review
soaratul’s picture

Issue summary: View changes
soaratul’s picture

StatusFileSize
new620.83 KB
soaratul’s picture

Issue summary: View changes

larowlan’s picture

Status: Needs review » Needs work

There's one remaining thread here that I think can be resolved by just removing that piece of code, it seems to work fine without it.

larowlan’s picture

Status: Needs work » Reviewed & tested by the community

@effulgentsia asked me to make that change and merge this so that @soaratul can continue with #3497530: Implement the "Publish All" button

  • larowlan committed ed9d69d4 on 0.x authored by soaratul
    Issue #3491459 by soaratul, larowlan, effulgentsia, lauriii, hooroomoo,...
larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 0.x - thanks!

lauriii’s picture

Issue summary: View changes
StatusFileSize
new23.44 KB

We might need some follow-ups for this if these don't already exist:

  1. The change icons are not using the correct colors. Based on https://www.figma.com/design/Ps3m4APGHIILsBrm0Uj31N/Experience-Builder?n..., components are supposed to be purple and global regions are supposed to be green.
  2. A page shows up categorized as a component. It should be categorized as a page.
  3. Instead of showing "Olivero global template" has been changed, the review panel should show which regions have been changed: https://www.figma.com/design/Ps3m4APGHIILsBrm0Uj31N/Experience-Builder?n...
  4. It doesn't seem to display a consistent color for a user. A single user should have the same color displayed for them to make it easy to identify changes made by one user.
wim leers’s picture

Status: Fixed » Needs work

Rather than creating follow-ups, let's just do another MR in this issue: to get the scope of this issue solved here.

jessebaker’s picture

Using the same numbers as #22

  1. The Figma isn't immediately clear. Is it correct that the categorization is as follows
    • Pages - grey, page icon (Although home page is show with a House icon)
    • Global Regions - green, cube icon (Figma also shows the diagonal 3 square icon in some places and refers to them as Global Sections)
    • Components - purple, diagonal 4 square icon
    • Elements - grey, various icons (we don't have a concept of elements yet)
  2. Should be a simple fix. I think the code currently falls back to the component icon for types it doesn't know and does not have a case for 'node'.
  3. I believe this will need a change to the data the endpoint returns - perhaps better suited to a follow up issue.
  4. This poses many questions. Where does the colour come from? Can users choose a colour? Is the colour stored somewhere or should it be randomly generated on page load? Should the colour come back as a property of the user and be determined on back end? What are the available colour options? Again, maybe a follow up for this.
lauriii’s picture

The Figma isn't immediately clear. Is it correct that the categorization is as follows

Correct 👍

Should be a simple fix. I think the code currently falls back to the component icon for types it doesn't know and does not have a case for 'node'.

Ah you're right that this is actually a node and it should use the CMS icon. I think we should also come up with a better fallback icon.

I believe this will need a change to the data the endpoint returns - perhaps better suited to a follow up issue.

+1 for a follow-up.

This poses many questions. Where does the colour come from? Can users choose a colour? Is the colour stored somewhere or should it be randomly generated on page load? Should the colour come back as a property of the user and be determined on back end? What are the available colour options? Again, maybe a follow up for this.

Sounds like something we should probably figure out in a follow-up.

effulgentsia’s picture

Issue tags: +sprint

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.

longwave’s picture

Opened #3501449: [Needs design] Decide how to assign colors to users for "review changes" for #22.4 / #24.4.

I think we could finish off #22.1 and #22.2 in this issue.

longwave’s picture

Status: Needs work » Needs review

MR!577 implements #24.1/#24.2 and the comment in #25.

  • Pages have a grey page icon.
  • Global regions have a green cube icon.
  • Nodes (CMS content) have a grey cylinder icon.

The last one wasn't available in Radix so I exported the SVG from Figma and added vite-plugin-svgr to be able to use SVGs as React components - looks like this is the first time we have needed to do this.

longwave changed the visibility of the branch 3491459-create-api-slice-for-pending-changes to hidden.

lauriii’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new39.06 KB

The icons and colors are working as expected now:

  • jessebaker committed 800b9b24 on 0.x authored by longwave
    Issue #3491459 by soaratul, larowlan, longwave, lauriii, effulgentsia,...
jessebaker’s picture

Status: Reviewed & tested by the community » Fixed

I've approved and merged !577 which resolves #24.1 and #24.2

Closing this issue. There are two follow ups for #24.3 and #24.4

#3500390: The pending changes API endpoint should list individual regions for global template changes and
#3501449: [Needs design] Decide how to assign colors to users for "review changes"

effulgentsia’s picture

Issue tags: -sprint

Status: Fixed » Closed (fixed)

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