Opened 9 years ago
Closed 8 years ago
#30201 closed enhancement (wontfix)
img_caption_shortcode_width should support percentage widths
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Media | Keywords: | good-first-bug has-patch needs-testing close |
Focuses: | Cc: |
Description
Currently, it assumes the returned value to be pixels.
Attachments (1)
Change History (17)
#2
@
9 years ago
I'm not exactly a veteran contributor or anything but I'm not certain that using a css3 function is the best idea for this as there's still a fair amount of people stuck using browsers that don't support css3 at all. Worried it wouldn't degrade gracefully for those users.
I prefer your suggestion of using margin/padding instead as the solution to add the extra space.
#4
@
9 years ago
Not possible to keep it purely within the css (I assume you mean a stylesheet) as it needs to output a user specified value.
@
9 years ago
This patch adds a right margin of 10px when a percentage width is returned from the filter. This is my first patch/contribution/trac usage so go easy :)
#7
@
9 years ago
- Keywords dev-feedback added
Isn't the margins added automatically when the alignleft and alignright classes are added? @hiwhatsup's solution seems to work nicely, however tinyMCE strips away the % from the shortcode, so that needs patching as well. the current regex is /width=['"]([0-9]*)['"] ?/ which matches just numbers, we need to amend that so that it matches numbers as well as %.
I think a new attr might be the best way, a 'caption_width_unit' one which defaults to px for backwards compatibility, and allows either blank (as default), 'px' or '%'. This way we could also add em or whatever later.
Thoughts?
#8
@
9 years ago
How would the percentage width be used? Limiting with max-width
(in combination with @media) seems to cover nearly all user cases?
Also, percentage width on a big screen might stretch it over the image size.
#9
follow-up:
↓ 10
@
9 years ago
IIRC, we were doing something with 50%
width (and float left / right). We used media queries to set lower and upper bounds.
#10
in reply to:
↑ 9
;
follow-ups:
↓ 11
↓ 15
@
9 years ago
- Keywords close added; dev-feedback removed
Replying to danielbachhuber:
There is no good way to use default percentage width without also setting min and max width. It will also force scaling of the image, possibly up-scaling in some cases.
Thinking this is best left for the theme to implement. I know it may need an !important
rule, but probably warranted here. Suggesting wontfix.
#11
in reply to:
↑ 10
;
follow-up:
↓ 12
@
9 years ago
Replying to azaozz:
I don't think that anyone is suggesting a default percentage width - more that they would like the option of setting a percentage if the usage called for it. The default should still be pixel based as defined by the theme image sizes.
Also I agree that percentage widths don't have many good usage cases unless the theme specifies a min-width/max-width but don't think we should totally disregard this idea because of that. That being said your the one here with most experience of developing WP so you're probably the one in the best position to make that decision :p
#12
in reply to:
↑ 11
;
follow-up:
↓ 13
@
9 years ago
Replying to williampatton:
I don't think that anyone is suggesting a default percentage width...
I meant supporting a percentage width for image captions as an option in TinyMCE, sorry for not making it clearer. It's not acceptable to have it work one way in the editor and another way on the front-end. Currently this is possible to do both in the editor and on the front-end with width: 50% !important
, etc. and IMHO this covers the user cases well.
#13
in reply to:
↑ 12
@
9 years ago
Replying to azaozz:
I meant supporting a percentage width for image captions as an option in TinyMCE, sorry for not making it clearer. It's not acceptable to have it work one way in the editor and another way on the front-end. Currently this is possible to do both in the editor and on the front-end with
width: 50% !important
, etc. and IMHO this covers the user cases well.
Ahh OK I get you. To be honest I hadn't realized there would be a discrepancy between editor and front-end. Being someone who's more comfortable using the text editor than the visual one I often overlook issues like that.
There are a handful of cases where allowing a percentage to be used would be handy but I'm starting to see your point and edging more towards your wontfix suggestion. Themes that set something like width:50% !important
definitely would cover most use cases - and since [caption]
can accept a custom class name I think that offers an acceptable way to set anything else that might be required.
I second the notion of 'wontfix' for this ticket but can't help thinking that there's got to be something that could be done for anyone that does need to be specify percentages for caption sizes... unfortunately I'm not really sure what that 'something that could be done' might be :(
#15
in reply to:
↑ 10
@
9 years ago
Replying to azaozz:
Thinking this is best left for the theme to implement. I know it may need an
!important
rule, but probably warranted here. Suggesting wontfix.
Ok so I have been assigned this ticket, but are we thinking its a wontfix? Happy to defer to your judgement on this @azaozz
Maybe something like this might work? Thoughts?
Since by default, the caption is 10 pixels greater than the width of the image. We'll need to add those 10px using CSS3 function calc(). I'm not how well this will work on older browsers. Another option is to add a margin or padding.
This is my first time here after attending @wcsf14. I wanted to ask before committing a diff and get some feedback for veteran committers and contributors.