#21092 closed enhancement (fixed)
Media uploader insert button doesn't respect the current post type
Reported by: | evansolomon | Owned by: | nacin |
---|---|---|---|
Milestone: | 3.5 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Media | Keywords: | needs-patch needs-codex |
Focuses: | Cc: |
Description
The media uploader's insert button always says "Insert Into Post", even if you're editing another post type. It's especially confusing if you're editing a custom post type that doesn't look anything like a post, like an image slider or product.
The button should ideally say "Insert into [singular post type]" -- for example, "Insert Into Page".
Attachments (6)
Change History (31)
#2
follow-up:
↓ 3
@
12 years ago
The approach in your patch works fine in English, but it's not suitable for l10n. We can't simply pass the post type name as a parameter to sprintf()
because in some languages the phrase "Insert into [post_type]" will differ depending on the post type.
We'll need to introduce a new post type label, probably called insert_into_post
, and use that. See #19257 for an example of a patch which introduces a new post type label.
#3
in reply to:
↑ 2
@
12 years ago
Replying to johnbillion:
We'll need to introduce a new post type label, probably called
insert_into_post
, and use that. See #19257 for an example of a patch which introduces a new post type label.
This seems a little different because we never want to use a default string; the default should incorporate the post type name, or at least that's the point of the ticket. One idea is to check for a specific label but never set a default, then fall back to "Insert into $post_type". I realize that reintroduces the l10n problem, but I don't think there's any other way to improve the defaults.
#4
follow-up:
↓ 5
@
12 years ago
WordPress defaults to the 'post' label for every other post type label. For l10n reasons, the behaviour of this label shouldn't be any different. Here's an example why:
Language X uses grammatical gender or some other funky language construct, and the entire phrase "Insert into post" can differ depending on the noun. If we default to the 'post' label for insert_into_post
then the phrase will always be grammatically correct.
If instead we try to build the phrase using the singular label as the noun, there's a chance the resulting string will be grammatically incorrect and possibly make no sense at all. It's better to default to the correctly localised version of "Insert into post" rather than an incorrectly localised version of "Insert into [custom post type]".
Your patch should include the insert_into_post
label for WordPress' built in Post and Page post types (in get_post_type_labels()
). It's then up to individual post types to include their own insert_into_post
label.
I realise this doesn't address what you are trying to achieve with this ticket (which is, for this button to always display the correct post type name for custom post types) but unfortunately that's how it is.
#5
in reply to:
↑ 4
@
12 years ago
Replying to johnbillion:
Your patch should include the
insert_into_post
label for WordPress' built in Post and Page post types (inget_post_type_labels()
). It's then up to individual post types to include their owninsert_into_post
label.
I realise this doesn't address what you are trying to achieve with this ticket (which is, for this button to always display the correct post type name for custom post types) but unfortunately that's how it is.
Fair enough, this makes sense. I took another swing at it in 21092.3.diff.
#6
follow-up:
↓ 7
@
12 years ago
Purely code review:
isset( $_GET ) && isset( $_GET['post_id'] )
There's no need to check to see if GET is set, You can simply do if ( isset( $_GET['post_id'] ) )
or !empty()
if that fits the use-case better
esc_attr__( $parent_type_obj->labels->insert_into_post )
esc_attr__
is a alias for esc_attr( __(...) )
so you should remove the __
from the function call
#7
in reply to:
↑ 6
@
12 years ago
Replying to dd32:
Nice catches, behold 21092.4.diff.
While we're at it, I'm not sure if the else
cases for the GET
and POST
params not existing ever happen. It seemed prudent to add it, but I haven't been able to find a case where it actually happens. Feedback on that would be welcome if anyone has it.
#8
follow-up:
↓ 9
@
12 years ago
What about it saying something more like "Insert into content" or something along those lines? It's already sort of confusing when next to the "Use as featured image" link (another issue entirely), so I wonder if it would be more effective to more clearly indicate that it is, at least by default, going into an editor instance.
#9
in reply to:
↑ 8
@
12 years ago
- Keywords ux-feedback added
Replying to helenyhou:
What about it saying something more like "Insert into content" or something along those lines? ... I wonder if it would be more effective to more clearly indicate that it is, at least by default, going into an editor instance.
Was thinking the same. Perhaps one of these:
- "Insert into content"
- "Send to editor"
- "Add"
- "Insert"
The button that opens that popup has "Insert" too.
#10
@
12 years ago
If we go with 21092.4.diff, insert_into_post
should fall back to an existing value (singular_name
seems appropriate) if it isn't set (see ticket:15576:7).
I like the idea of "Insert into content" / "Send to editor" though, sounds much simpler.
Seems like we already had "Send to editor" at some point earlier, that string still exists in TinyMCE help:
http://core.trac.wordpress.org/browser/tags/3.4/wp-includes/js/tinymce/wp-mce-help.php#L223
#11
follow-up:
↓ 13
@
12 years ago
"Send to editor" might be a little ambiguous, since there's also image editing in core. Maybe the logical combination of the two, if headed that route - "Insert into editor".
#12
in reply to:
↑ 1
@
12 years ago
Replying to evansolomon:
20192.diff ignores the "Insert Into Post" strings in
_insert_into_post_button()
because the function isn't used anywhere.
Related: #20427
#13
in reply to:
↑ 11
@
12 years ago
Replying to helenyhou:
"Send to editor" might be a little ambiguous, since there's also image editing in core. Maybe the logical combination of the two, if headed that route - "Insert into editor".
I don't think "Insert into editor" clarifies ambiguity in "Send to editor" -- the multiple editors issue still exists. While technically the button does sense an image to an editor, I think we'd be remiss to not talk about the post (or whatever content type) since that's the public-facing thing that someone is making on that page.
#15
@
12 years ago
Update on ~ (as it's been about four months):
Button still says Insert into post
Currently hasn't been resolved.
#16
@
12 years ago
For 3.5, this would likely be best as a simple filter, like apply_filters( 'insert_into_post', __( 'Insert into post' ), $post );
.
#17
@
12 years ago
21092.patch lets you change all strings, also insertIntoPost
.
#18
@
12 years ago
I can go for this. It seems like 21092.patch should side-step filtering $settings, lest something get broken, and focus squarely on labels?
#19
@
12 years ago
- Owner set to nacin
- Resolution set to fixed
- Status changed from new to closed
In 22702:
#23
@
12 years ago
- Keywords needs-patch added; has-patch removed
- Resolution fixed deleted
- Status changed from closed to reopened
On post-new.php
:
Notice: Undefined variable: post_id in /wp-admin/edit-form-advanced.php on line 29
#25
@
12 years ago
- Keywords needs-codex added
New filters [22702]:
- media_view_settings
- media_view_strings
20192.diff ignores the "Insert Into Post" strings in
_insert_into_post_button()
because the function isn't used anywhere.