Skip to content

fix: actions menu for envelope#13296

Open
madbob wants to merge 1 commit into
nextcloud:mainfrom
madbob:issue_12586
Open

fix: actions menu for envelope#13296
madbob wants to merge 1 commit into
nextcloud:mainfrom
madbob:issue_12586

Conversation

@madbob

@madbob madbob commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

I've simplified a bit the management of actions menu in EnvelopeSkeleton, to handle the many involved conditions.

Notes:

  • the ugly setTimeout in handleActionsUpdateOpen is to delay a bit the status change for the closed menu; without it, the flickering menu mentioned in Action menu flickers for some actions #12586 cannot be really fixed. I've already opened an issue for nextcloud-vue in order to have a proper way to manage this
  • almost unrelated to the target issue - but still related to actions menu - are the changes in MessageHTMLBody. The (untracked) issue is that NcActions registers a global handler to intercept clicks made on whole document and determines if the opened menu has to be closed, but clicks occurring over the iframe of MessageHTMLBody are "absorbed" by the iframe itself and do not bubble to the rest of the document. This change is intended to pre-intercept those clicks and properly deliver them to that global callback: now action menus are closed also when clicking on the HTML body of an email (which often occupies the large portion of the screen). If those changes seem too unrelated to the scope of this PR, I can split them in a different dedicated PR
  • wandering up and down EnvelopeSkeleton I've found a few props that are never set by anyone and I guess can be removed. forceDisplayActions has been here suppressed, while counterNumber and counterType are still there. The commit introducing them (0b15b42) provides no hints about their scope. If no-one has any suggestion about their meaning and intended usage, I can remove them within this same PR

Fixes #12586

@ChristophWurst

Copy link
Copy Markdown
Member

Thanks! Will try to review soon 🤞

this.hovered = false
},

hideActions() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

method still used on L39

Comment thread src/components/EnvelopeSkeleton.vue Outdated
Comment on lines 355 to 357
handleBlur(e) {
// do not hide if focus is kept within
if (this.$refs['list-item'].contains(event.relatedTarget)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

e vs event

Comment thread src/components/MessageHTMLBody.vue Outdated
Comment on lines +153 to +164
// Clicks occurring into the iframe are forced to bubble to the
// document, to be intercepted by other functions (notably: the
// global handler of NcActions used to close menus)
iframeDoc.addEventListener('click', (e) => {
const replicatedClick = new MouseEvent('click', {
bubbles: true,
cancelable: true,
view: window,
})

document.dispatchEvent(replicatedClick)
})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm a bit skeptical about the impact of this change. Maybe this should be a separate PR.

Comment thread src/components/EnvelopeSkeleton.vue Outdated
:id="anchorId || undefined"
:aria-label="linkAriaLabel"
class="list-item__anchor"
ref="mainCard"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

unused ref

Signed-off-by: Roberto Guido <info@madbob.org>
@madbob

madbob commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

I've overwritten the previous commit, following the wise notices of @ChristophWurst

Highlights:

  • removed the unused ref to mainCard, introduced by one of my previous tries and forgot in the code
  • fixed e / event reference in handleBlur(): I have no idea how I missed that, as I tested it and it worked. Eventually I messed up everything moving code around
  • entirely removed the @keydown.esc callback assignation, as it is already handled by NcActions and seems to be redundant here. Important note: I've tested keyboard navigation, but an extra validation by an accessibility-focused person would be really appreciated
  • removed all changes to MessageHTMLBody, to be submitted in a dedicated PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Action menu flickers for some actions

2 participants