Overview

We now have a contextual menu for components thanks to #3458723: Implement context menu for hovered component. However, this is currently exposed via an icon that is displayed on the top left corner of the component. Let's experiment if we could expose this by right clicking the component both either within the preview or the component hierarchy list.

User interface changes

CommentFileSizeAuthor
#30 Untitled.gif154.74 KBgauravvvv
#23 hover broken.gif149.65 KBwim leers
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

lauriii created an issue. See original summary.

fazilitehreem’s picture

Assigned: Unassigned » fazilitehreem

fazilitehreem’s picture

Assigned: fazilitehreem » Unassigned
Status: Active » Needs review
lauriii’s picture

Assigned: Unassigned » jessebaker

Based on the recording looks great! 👏 @fazilitehreem if you have a moment, it would be helpful if you could rebase this. 😊

Assigning for @jessebaker to review!

jessebaker’s picture

Thanks @fazilitehreem I've left a comment on the MR.

jessebaker’s picture

@fazilitehreem keep an eye on MR !93 - hopefully we will merge it soon.

fazilitehreem’s picture

Assigned: jessebaker » fazilitehreem
Status: Needs review » Needs work

I'll continue to work on this issue

wim leers’s picture

So is this issue's MR dependent on MR 93?

fazilitehreem’s picture

@Wim This ticket is dependent on MR !93 as it uses the ContextMenu, which handles right-click functionality. Therefore, this requires the changes introduced in MR !93. This also needs some work around with ContextMenu implemented.

jessebaker’s picture

fazilitehreem’s picture

wim leers’s picture

Title: Allow opening the contextual menu by right clicking a component » [PP-1] Allow opening the contextual menu by right clicking a component

#10: thanks! Updated issue state to reflect that, and made MR 96 depend on MR 93 👍

fazilitehreem’s picture

@jessebaker I did some work around with to check how it works with the current requirements. But with the limitation of , it requires an area to trigger the context menu. On adding the area, it blocks the other MouseEvents ( grab and click ) functionality of the component.
I tried with other ways like onOpenChange or to pass the reference of the actual component but still the blocker persists. To keep the all the MouseEvents on a single components this can be handle with custom code as done earlier in !MR 36 .
Was the added on purpose to add some required feature. Can we handle it with the menu?. Need your thoughts here. thanks

https://www.loom.com/share/f5c38bbadde245f1ba22593a6070547d?sid=5274b17c...

fazilitehreem’s picture

Assigned: fazilitehreem » Unassigned
wim leers’s picture

Title: [PP-1] Allow opening the contextual menu by right clicking a component » Allow opening the contextual menu by right clicking a component

#3460783: Implement component states landed yesterday, so this is now unblocked 😊

fazilitehreem’s picture

Status: Needs work » Needs review
wim leers’s picture

Status: Needs review » Needs work

There are unaddressed reviews on the MR from 2 weeks ago, and it's not passing tests. There's many upstream changes that need to be merged in from the past 2 weeks.

fazilitehreem’s picture

Status: Needs work » Needs review
wim leers’s picture

Status: Needs review » Needs work

Failing the UI eslint CI job and tests. 😅

omkar-pd made their first commit to this issue’s fork.

omkar-pd’s picture

Solved Merge conflicts, Fixed UI Eslint and other tests, but Can access XB UI and do basic interactions keeps failing. :(

wim leers’s picture

Issue tags: +Needs screenshots
StatusFileSize
new149.65 KB

Ran it locally, can confirm the hard failure on:

    // After hovering, the component should be outlined for both small and large viewports.
    cy.get('[data-xb-component-outline]')

… but that looks like a legitimate test failure: the expectations of the test need to be updated.

Notice the

    // Enter the iframe to find an element in the preview iframe and hover over it.
    cy.getIframeBody().find('[data-component-id="experience_builder:my-hero"] h1')
      .first()
…

a few lines higher.


And so I tried this branch locally and … sure enough: it is broken. The appropriate outline only appears on click anymore, not on hover:

gauravvvv’s picture

Assigned: Unassigned » gauravvvv
gauravvvv’s picture

Assigned: gauravvvv » Unassigned
fazilitehreem’s picture

Assigned: Unassigned » fazilitehreem
fazilitehreem’s picture

Assigned: fazilitehreem » Unassigned
Status: Needs work » Needs review
wim leers’s picture

Status: Needs review » Needs work

I bet merging in #3467859: Account for GitlabCI intermittent path differences in SDC CSS test will fix this 🤞

P.S.: you can record the screenshot/GIF I requested using https://www.cockos.com/licecap/ :)

gauravvvv’s picture

Status: Needs work » Needs review
gauravvvv’s picture

Issue tags: -Needs screenshots
StatusFileSize
new154.74 KB

Here is the MR screen-recording, All tests are passing now. MR is ready for review

wim leers’s picture

Issue summary: View changes
Issue tags: +Usability

👏

jessebaker’s picture

Status: Needs review » Needs work

I've reviewed - I found one small bug described on the MR and it needs some tests.

wim leers’s picture

Issue tags: +Needs tests
fazilitehreem’s picture

Assigned: Unassigned » fazilitehreem
fazilitehreem’s picture

Assigned: fazilitehreem » Unassigned
jessebaker’s picture

Assigned: Unassigned » utkarsh_33

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

utkarsh_33’s picture

The problem was related to how the position is being calculated.
By introducing isPositionUpdated and using it to control when the context menu is rendered, the new code ensures that the menu only appears after its position has been recalculated for the new location.
I'll now add some tests for this.

utkarsh_33’s picture

Assigned: utkarsh_33 » jessebaker
Status: Needs work » Needs review

I think a bad merging has led to the failures of CI as it was passing here.I am assigning it to Jesse to review the changes that i have pushed along with the tests.

jessebaker’s picture

Assigned: jessebaker » Unassigned
Status: Needs review » Fixed
Issue tags: -Needs tests

Merged! Great work everyone, thank you!

jessebaker’s picture

kristen pol’s picture

Yay! Duplicate and move up/down and delete! 😍

When trying to edit, the props form is hanging but I assume that's a different problem. I know there is a performance issue that's being worked on.

kristen pol’s picture

I have a video that shows the behavior I'm seeing on the right sidebar props form hanging (from when I was testing the dragging behavior):

https://youtu.be/j46NzjwEfp8

kristen pol’s picture

Clearing the cache seems to have "fixed" the hanging props form issue (even though this was a fresh install).

Status: Fixed » Closed (fixed)

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