Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.I was just working on another patch an noticed that the progress.gif image added here #653620: Bring progressbar to the modern era doesn't properly tile. It's 20x20px, but uses 1.5em for line-height which is based on a 12px font-size. If the font-size is any larger, the image looks messed up because it has a slightly light shade for the top row of pixels.
There's 2 options to fix this:
1 - Fix the image (get rid of the lighter shade)
2 - Stop using ems and set the height to 20px
I've attached screenshots of the problem.
This is the code that causes it.
.progress .filled {
background: #0072b9 url(../../misc/progress.gif);
height: 1.5em;
width: 5px;
}| Comment | File | Size | Author |
|---|---|---|---|
| #12 | progress-5.gif | 5.71 KB | Jacine |
| #12 | progress-10.gif | 5.73 KB | Jacine |
| #10 | progress-5%.gif | 5.71 KB | Jacine |
| #10 | progress-10%.gif | 5.73 KB | Jacine |
| #10 | comparison.png | 5.41 KB | Jacine |
Comments
Comment #1
sunUgly :)
Comment #2
bleen CreditAttribution: bleen commentedfixed
Comment #3
JacineHey, thanks @bleen.
It's better, but I'm still seeing a problem. :(
Comment #4
bleen CreditAttribution: bleen commentedhuh? I don't see how thats possible ... take a look at the gif in #2 on its own. Do you still see that light colored strip?
here is what I have with the new gif:
Comment #5
jbrown CreditAttribution: jbrown commentedThe image is lighter at the top (vertical gradient) - so it can't tile vertically.
I think the height needs to be specified explicitly to be the same as the image.
If you zoom in text only, the same problem will still occur.
Comment #6
bleen CreditAttribution: bleen commentedok ... this .gif has no vertical gradient
Comment #7
Jeff Burnz CreditAttribution: Jeff Burnz commentedThe reworked gif in #6 solves the issues in that you can scale the progress bar by increasing the font size. The down side is that it doesn't look quite as good as the original.
I suspect we need to decide if a scaling progress bar wins over better looking fixed height progress bar. I assume that since the original CSS uses em's the intent was to allow it to scale and changing that would require a UX review.
From an accessibility point of view its probably better that it scales, but its pretty big to start with and I'm not entirely sure if it matters.
In any case the #6 gif solves the problem. The image below is with the font size well over 200%. You can see it looks fine.
Comment #8
jbrown CreditAttribution: jbrown commentedI don't think we need to support text-only zooming. The gradient background on the active menu on the toolbar suffers a similar problem as it uses CSS sprites.
Comment #9
Jeff Burnz CreditAttribution: Jeff Burnz commentedEveryone here knows we can do this (fix the height of the progress bar), but why exactly wouldn't we want to support text zooming? This could be useful for someone with very poor vision.
If your point is that we don't need to support text resizing et al, then that is wrong - Drupal 7 is to be WCAG AA compliant. Everywhere else we have worked hard to ensure proper text zooming works.
Your patch is actually removing a feature in Drupal, whereas the gif in #6 is working to preserve this feature and simply fixes a display issue.
RTBC for the gif in #6.
Comment #10
Jacine@bleen Sorry! I didn't realize there was a gradient. I didn't mean to cause you extra work :(
Anyway, we don't have to loose the gradient OR go with a fixed height. Here's 2 versions of progress.gif that have tile-able gradients. I'm not positive what the transparency level was applied in the first place, so there are 2 versions attached: one, with 5% and one with 10%.
comparison.png shows all 3 version side by side.
Comment #11
sun10% looks much fancier to me here.
Comment #12
JacineApparently adding a % to a file name is no good. Oops. LOL.
Comment #13
bleen CreditAttribution: bleen commented10% looks nicer
Comment #14
Jeff Burnz CreditAttribution: Jeff Burnz commentedAwesome Jacine, can't beat the best of both worlds. Just to clarify we need 200% text re-size support - these clearly do that. +1.
Comment #15
yoroy CreditAttribution: yoroy commentedYup, good to have please.
Comment #16
Dries CreditAttribution: Dries commentedAlright. Committed progress-10.gif as misc/progress.gif.
Comment #17
jbrown CreditAttribution: jbrown commentedThis shouldn't have been attributed to me: http://drupal.org/cvs?commit=417524
On a separate note, Jeff - I didn't say "we don't need to support text resizing". I said "I don't think we need to support text-only zooming". Please don't misrepresent me.
Text-only resizing is not compatible with CSS sprites (which is used extensively), so it isn't something that Drupal supports. Try the autocomplete widget with a text-only zoom to see what I mean.
Comment #18
Jeff Burnz CreditAttribution: Jeff Burnz commentedjbrown, its more accurate to say that Drupal has bugs that need to be fixed - if we have sprites that aren't working with proper text resizing (and we do, I know it) then that needs to be fixed.
Comment #20
kika CreditAttribution: kika commentedsubscribing to be up to date with partly my baby