Opened 12 years ago
Closed 12 years ago
#22541 closed defect (bug) (fixed)
Media icon doesn't work in distraction free writing mode
Reported by: | ericmann | Owned by: | koopersmith |
---|---|---|---|
Milestone: | 3.5 | Priority: | normal |
Severity: | blocker | Version: | 3.5 |
Component: | Media | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
While the Add Media button works in the regular post editor, launching the DFW mode is a different story. The button is available in the toolbar, but neither clicking it or using the keyboard shortcut (Ctrl + Shift + M) launches the media dialog.
Attachments (4)
Change History (25)
#1
@
12 years ago
- Milestone changed from Awaiting Review to 3.5
- Severity changed from critical to blocker
#3
in reply to:
↑ 2
@
12 years ago
Replying to azaozz:
However DFW's z-index trumps the new media modal z-index. So, increase the media modal or decrease the DFW?
Increase the modal. DFW is designed to fade in over everything. But, the modal is just that — a modal dialog. It should appear over everything. That includes the existing z-index of DFW.
Also, by raising the z-index on the media modal, we can help plugin compatibility. By lowering DFW, it can only cause problems.
#4
follow-up:
↓ 5
@
12 years ago
This ticket makes me wonder if anyone else might be summoning the old modal in the same way. Granted, the medialib function is rather esoteric in its current state, but does anyone think that may be a concern?
#5
in reply to:
↑ 4
;
follow-ups:
↓ 6
↓ 7
@
12 years ago
22541.patch seems to work but needs some more testing (only did quick test) and is kind of hacky.
We can add the insert-media
class to DFW's media button as the original event is attached to the body or even have a one-line method to open the modal that can be called with onclick
, etc.
However the original button also has data-editor="..."
which would be harder to set in these cases. Not sure why it's there, seems wpActiveEditor
can be used instead.
cc: koopersmith
Replying to nacin:
We could use the old class on the link which would keep any JS accessing it that way working. Think .insert-media
was needed as the UI changed from a link to a button.
#6
in reply to:
↑ 5
@
12 years ago
Replying to azaozz:
22541.patch seems to work
Yep. +1.
Side note: The following styles also have a z-index greater than 160000:
wp-admin.css
- .wp-full-overlay - set to 500000
- #customize-container - set to 500000
editor.css
- #wp_editbtns, #wp_gallerybtns - set to 999998
- .fullscreen-fader - set to 200000
#7
in reply to:
↑ 5
;
follow-up:
↓ 8
@
12 years ago
Replying to azaozz:
However the original button also has
data-editor="..."
which would be harder to set in these cases. Not sure why it's there, seemswpActiveEditor
can be used instead.
data-editor
is used to track the instance of the media frame that the button will open. It's possible to click a button without switching wpActiveEditor
. That said, we should probably default that property to the wpActiveEditor
if it's empty or not provided.
#8
in reply to:
↑ 7
;
follow-up:
↓ 11
@
12 years ago
Replying to lessbloat:
Side note: The following styles also have a z-index greater than 160000:
- .wp-full-overlay - set to 500000
- #customize-container - set to 500000
Thinking these are ok, the customizer should be higher than the media modal?
editor.css
- #wp_editbtns, #wp_gallerybtns - set to 999998
These should ideally be lower than the modal.
- .fullscreen-fader - set to 200000
This can be higher, used only to fade in/out DFW.
Replying to koopersmith:
... It's possible to click a button without switching
wpActiveEditor
. That said, we should probably default that property to thewpActiveEditor
if it's empty or not provided.
wpActiveEditor
is (should be) set on mousedown, so any click in the editor wrapper would set it to the current instance id before the click
event fires. Of course having data-editor
there and falling back to wpActiveEditor
wouldn't hurt.
#9
@
12 years ago
In 22541-2.2.patch (the name should have been -2.patch... Chrome messed up):
- Introduce
wp.media.editor.open()
. - Call it from the Add Media button, wp-fullscreen.js for DFW and 'WP_Medialib' command in MCE (used on the old fullscreen).
- Reduce the z-index for the popup buttons in MCE (see above).
#10
@
12 years ago
- Keywords has-patch added; needs-patch removed
- Owner set to koopersmith
- Status changed from new to reviewing
#11
in reply to:
↑ 8
;
follow-up:
↓ 12
@
12 years ago
Replying to lessbloat:
Side note: The following styles also have a z-index greater than 160000:
Any reason why we're using 160000 in particular?
Replying to azaozz:
- .wp-full-overlay - set to 500000
- #customize-container - set to 500000
Thinking these are ok, the customizer should be higher than the media modal?
Using media inside the customizer is a valid use case. If the customizer is using customize-loader and is injected over the content using an iframe, the iframe should be positioned above any media modal on the outer page. However, the media modal should be able to work inside customize.php, which (unfortunately) currently applies .wp-full-overlay
to the main containing <div>
. We should either lower the z-index entirely, or just on customize.php using customize-controls.css.
#12
in reply to:
↑ 11
@
12 years ago
Replying to koopersmith:
Replying to lessbloat:
Side note: The following styles also have a z-index greater than 160000:
Any reason why we're using 160000 in particular?
Ah. The main body of DFW is around 150000 (which wasn't noted earlier).
#13
@
12 years ago
- Keywords commit added
22541.3.diff builds off of 22541-2.2.patch:
- The main page and DFW now share a media instance.
- More detailed existence checks for
wp.media.editor
. - Revises logic in
wp.media.editor.open
, which now returns the specified instance. - Lowers the main z-index to
160000
and the backdrop z-index to159900
. This optimizes for placing content above the modal as opposed to between the modal and the backdrop.
#14
@
12 years ago
22541.3.diff works well with one small exception: in DFW the original editor id is stored in settings.editor_id
, 22541-4.patch passes that to wp.media.editor.open()
. Other than that, it's identical to 22541.3.diff.
Commit +1.
#16
@
12 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
The wp_editbtns and wp_gallerybtns buttons don't show up in DFW any longer.
#18
@
12 years ago
Yeah, we need the buttons to be higher than DFW but lower than the medial modal. Confirmed that works.
We can make the button work easily in wp-fullscreen.js:
However DFW's z-index trumps the new media modal z-index. So, increase the media modal or decrease the DFW?