-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great! Just one minor thing with the progress bar, then we 🚢
@@ -145,7 +171,7 @@ input[type=range]:focus { | |||
} | |||
|
|||
.atom--audio__progress-bar input[type=range]::-moz-range-progress { | |||
background-color: #C70000; | |||
background-color: #C70000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this need to be the pillar colour too?
border-radius: 50%; | ||
margin-top: -4px; | ||
} | ||
|
||
// these non-standard pseudo selectors can't be comma-separated - so much duplication :( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where SASS becomes useful:
$pillars: (
news: $c-pillar--news,
arts: $c-pillar--arts,
sport: $c-pillar--sport,
opinion: $c-pillar--opinion,
lifestyle: $c-pillar--lifestyle
);
@mixin thumb($property) {
.atom--audio__progress-bar input[type=range]::${property} {
@each $pillar, $colour in $pillars {
.garnett--pillar-${pillar} &,
.content--pillar-${pillar} & { background: $colour; }
}
}
@include thumb(-moz-range-thumb);
@include thumb(-webkit-slider-thumb);
@include thumb(-ms-thumb);
Not 100% sure of the syntax though 🤔
@@ -98,7 +98,8 @@ export default (componentType: ComponentType) => ({ ophan, dom, viewport }: Serv | |||
}; | |||
|
|||
const showProgress = (audio: Audio, percentPlayed: Number, now: Number, played: string) => { | |||
const gradientDescription = `linear-gradient(to right, #C70000 ${percentPlayed}%, #afafaf ${percentPlayed}%)`; | |||
// TODO: make the input control's background honour the pillar colour - meanwhile use the 'news' default | |||
const gradientDescription = `linear-gradient(to right, #e00000 ${percentPlayed}%, #afafaf ${percentPlayed}%)`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will effectively override the definition you set in the CSS. What I suggest instead is that you parse it on start, e.g. assuming a progressBar
reference to the DOM element:
const styles = getComputedStyle(progressBar);
const defaultValue = '#c70000';
const colorRegex = /##[\w\d]{6}/;
const pillarColour = (styles.getPropertyValue('background') || defaultValue).match(colorRegex);
// pillarColour[0] has the colour.
With CSS variables, this will be way easier:
const styles = getComputedStyles(document.body);
const pillarColour = styles.getPropertyValue('--c-main') || defaultValue;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful!
First pass to update the play/pause button and the slider thumb control to use the pillar colour.
We still have to find a way to make the progress bar apply the correct colour (or choose a neutral one) for the 'played' section. It's set by javascript and currently hard-coded to match the news pillar.
Before
After