Paths

Table of Contentst

Differential D25764

Leave fullscreen editor when displaying "Upload File" and "Meme" dialogs
AcceptedPublic
Actions

Authored by aklapper on Aug 4 2024, 00:19.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15901: "Upload File" and "Meme" dialogs display behind fullscreen editor

Summary

When being in fullscreen editor mode, the Upload File and Meme dialogs get opened behind the editor and thus are invisible.
Given the options listed in https://secure.phabricator.com/T8200, pop out of fullscreen when these buttons get clicked, as this is easiest to implement.

Closes T15901

Test Plan

Go to any item which allows commenting, e.g. a task.
In the toolbar of the textarea, click the Fullscreen Mode button.
Click the Upload File or Meme button in the toolbar.
Check if the dialog is displayed.
Repeat the test in normal mode (not "Fullscreen mode") and see that it still works.

Diff Detail

Repository

rP Phorge

Branch

arcpatch-D25764_1

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 1676
Build 1676: arc lint + arc unit

Event Timeline

aklapper created this revision.Aug 4 2024, 00:19

Owners added a reviewer: O1: Blessed Committers.Aug 4 2024, 00:19

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptAug 4 2024, 00:19

aklapper requested review of this revision.Aug 4 2024, 00:19

Harbormaster completed remote builds in B1497: Diff 2326.Aug 4 2024, 00:19

Thanks. Have you already tried with just CSS? Any issue in that? Maybe something like this:

.jx-client-dialog {
  z-index:50;
}

Bonus point: double-check notifications

This entire method looks very hacky to me. I guess at the bottom of every useful UI, there's a bunch of if statements that map icon names to code.

webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js
311	Looks like you don't need to check `edit_mode` before setting it. Also note `root` vs `edit_root`?

In D25764#20704, @valerio.bozzolan wrote:

Thanks. Have you already tried with just CSS? Any issue in that?

https://secure.phabricator.com/T8200#116651

Remove unneeded if statements.

Sorry, I do *not* understand the exact difference between root and edit_root - both make Phorge behave as expected (leave fullscreen)...

Harbormaster completed remote builds in B1501: Diff 2337.Aug 5 2024, 16:33

Untested but seems good to me, maybe nice to add a couple of

// Deactivate full-screen mode because it will cause sub-popup visibility problems.

Cigaryno added a project: Remarkup.Dec 29 2024, 16:43

It works on my computer. Thaaaanks \o/

I will just integrate the comment ( https://we.phorge.it/D25764#21610 ) - since I've obtained a kind live "ok yeah whatever bro" from aklapper lol

This revision is now accepted and ready to land.Sun, Feb 2, 15:36

add little cute comment and newline, to make it clear that the comment belongs only to that line, and really nothing else lol

valerio.bozzolan edited the test plan for this revision. (Show Details)Sun, Feb 2, 15:39

Harbormaster completed remote builds in B1676: Diff 2609.Mon, Feb 3, 08:00

Revision Contents
Changeset List

Path

Size

Packages

webroot/

rsrc/

js/

core/

behavior-phabricator-remarkup-assist.js

6 lines

O1: Blessed Committers

Diff 2609

View Options

webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js

Leave fullscreen editor when displaying "Upload File" and "Meme" dialogsAcceptedPublicActions