Closed Bug 991945 Opened 7 years ago Closed 7 years ago

Fix modern theme fallout from bug 649490 (Horizontal html5 audio/video volume control)

Categories

(SeaMonkey :: Themes, defect)

defect
Not set
normal

Tracking

(seamonkey2.25 affected, seamonkey2.26 fixed, seamonkey2.27 fixed, seamonkey2.28 fixed)

RESOLVED FIXED
seamonkey2.28
Tracking Status
seamonkey2.25 --- affected
seamonkey2.26 --- fixed
seamonkey2.27 --- fixed
seamonkey2.28 --- fixed

People

(Reporter: neil, Assigned: neil)

References

Details

(Keywords: modern)

Attachments

(3 files)

I actually saw bug 649490 land, but forgot to follow up on it, and nobody else noticed until Patrick Dempsey pointed out that Modern's videocontrols.css is now broken.
Attached patch Proposed patchSplinter Review
I didn't quite follow Patrick's suggestions.
The volumeStack now needs no themeing at all.
The volume control's height changes to a width, and I also halved it to match Classic. I switched the width and height on the volume control's thumb too of course.
The volumeBackgroundBar is now just volumeBackground, and also it's the margin's values that now need to be switched around.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #8401617 - Flags: review?(philip.chee)
Attachment #8401617 - Flags: review?(iann_bugzilla)
Of course further work could be done to make it look more like a volume control, but this at least unbreaks the theme.
Summary: Fix modern theme fallout from bug 649490 → Fix modern theme fallout from bug 649490 (Horizontal html5 audio/video volume control)
Comment on attachment 8401617 [details] [diff] [review]
Proposed patch

>  .volumeControl {
> -  min-height: 64px;
> +  width: 32px;
I prefer the width to be wider (say 64px) but in anycase r=me
Attachment #8401617 - Flags: review?(philip.chee) → review+
Keywords: modern
Version: unspecified → Trunk
Blocks: 992294
This is with a triangular volume control, see what you think?
Attachment #8402198 - Flags: review?(philip.chee)
Attachment #8402198 - Flags: review?(iann_bugzilla)
Comment on attachment 8402198 [details] [diff] [review]
Isosceles triangle

(In reply to neil@parkwaycc.co.uk from comment #4)
> This is with a triangular volume control, see what you think?
Hmm I prefer Patricks version in Bug 992294. It's less fragile.

.volumeBackground {
background-image: linear-gradient(to bottom right, rgba(255,255,255,0), rgba(255,255,255,0) 50%, rgba(255,255,255,.5) 51%, rgba(255,255,255,.5));
}

Adding a margin makes it look better

.volumeBackground {
margin: 5px 4px;
background-image: linear-gradient(to bottom right, rgba(255,255,255,0), rgba(255,255,255,0) 50%, rgba(255,255,255,.5) 51%, rgba(255,255,255,.5));
}
> (In reply to neil@parkwaycc.co.uk from comment #4)
> > This is with a triangular volume control, see what you think?
> Hmm I prefer Patricks version in Bug 992294. It's less fragile.

<NeilAway>	wonders what RattyAway means by "fragile"
<RattyAway>	NeilAway: it means I don't understand how it works
<NeilAway>	RattyAway: http://apps.eky.hk/css-triangle-generator/
Comment on attachment 8402198 [details] [diff] [review]
Isosceles triangle

(In reply to neil@parkwaycc.co.uk from comment #4)
> Created attachment 8402198 [details] [diff] [review]
> 991945.diff
> 
> This is with a triangular volume control, see what you think?
Looks rather ugly. Sorry.
Attachment #8402198 - Flags: review?(philip.chee) → review-
Attachment #8405909 - Flags: review?(philip.chee)
Attachment #8405909 - Flags: review?(iann_bugzilla)
Attachment #8402198 - Attachment description: 991945.diff → Isosceles triangle
FYI the first triangle is isosceles because Classic uses an isosceles triangle, while the second triangle is a right-angled triangle to match Patrick's suggestion (but using simpler CSS).
Comment on attachment 8405909 [details] [diff] [review]
Right-angled triangle

> -  min-height: 64px;
> +  width: 32px;
Looks good.

> -  min-width: 20px;
> +  min-height: 20px;
> +  min-width: 11px;
The scale thumb could be narrower, say 8px?

> +.volumeBackground {
...
> +  border-top-width: 16px;
> +  -moz-border-end-width: 24px;
What works for me is:

> border-top-width: 16px;
> -moz-border-end-width: 32px;

r=me with the above fixed plus please add a comment containing a pointer to
http://appendto.com/2013/03/pure-css-triangles-explained/
Attachment #8405909 - Flags: review?(philip.chee) → review+
(In reply to Philip Chee from comment #10)
> > -  min-width: 20px;
> > +  min-height: 20px;
> > +  min-width: 11px;
> The scale thumb could be narrower, say 8px?
Yes; of course I have to reduce the border radius to 4px.

> > +.volumeBackground {
> What works for me is:
> 
> > border-top-width: 16px;
> > -moz-border-end-width: 32px;

I'd like you to choose between three versions of the 2:1 aspect ration:

.volumeBackground {
  margin: 6px 0px;
  border: 0px solid transparent;
  border-top-width: 16px;
  -moz-border-end-width: 32px;
  -moz-border-end-color: #B1BBC5;
}

.volumeBackground {
  margin: 7px 2px;
  border: 0px solid transparent;
  border-top-width: 14px;
  -moz-border-end-width: 28px;
  -moz-border-end-color: #B1BBC5;
}

.volumeBackground {
  margin: 8px 4px;
  border: 0px solid transparent;
  border-top-width: 12px;
  -moz-border-end-width: 24px;
  -moz-border-end-color: #B1BBC5;
}
Flags: needinfo?(philip.chee)
Attachment #8401617 - Flags: review?(iann_bugzilla)
Attachment #8402198 - Flags: review?(iann_bugzilla)
Comment on attachment 8405909 [details] [diff] [review]
Right-angled triangle

I think the following one looks best:
.volumeBackground {
  margin: 6px 0px;
  border: 0px solid transparent;
  border-top-width: 16px;
  -moz-border-end-width: 32px;
  -moz-border-end-color: #B1BBC5;
}
Attachment #8405909 - Flags: review?(iann_bugzilla) → review+
Pushed comm-central changeset 8067ac095355.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: needinfo?(philip.chee)
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.28
Comment on attachment 8405909 [details] [diff] [review]
Right-angled triangle

[Approval Request Comment]
Regression caused by (bug #): Bug 649490 - Horizontal html5 audio/video volume control
User impact if declined: Unusable volume control in the media controls of <video> elements
Testing completed (on m-c, etc.): tested on comm-central
Risk to taking this patch (and alternatives if risky): No risk, feature was already broken.
String changes made by this patch: None.

a=me for a=comm-aurora and a=comm-beta.
Attachment #8405909 - Flags: approval-comm-beta+
Attachment #8405909 - Flags: approval-comm-aurora+
Duplicate of this bug: 992294
You need to log in before you can comment on or make changes to this bug.