search

PRX / Play-Angular

Embed player
MIT License
11 stars 7 forks source link

Add basic infrastructure for generic overlays on player #156

Closed cqr closed 7 years ago

cqr commented 7 years ago

includes some minor vertical responsiveness responsive

as well as infrastructure to allow us to support ad takeovers, endslate, and a redesigned share overlay, for both the large version: largeoverlay

and small version of the player: smalloverlay

cqr commented 7 years ago

This PR includes CSS vars which there is no way to make angular-cli work with so needs to be manually handled before merging. I wanted to leave them in place in case changes were requested

gcampo88 commented 7 years ago

looks a little funky for playlists! margin collapses between the scrub bar and the top ep.

image https://play.prx.org/e?uf=http:%2F%2Ffeeds.99percentinvisible.org%2F99percentinvisible&sp= vs. image

http://play.prx.dev/e?uf=http:%2F%2Ffeeds.99percentinvisible.org%2F99percentinvisible&sp=

binarynate commented 7 years ago

Hello, @gcampo88 and @cavis ! I just joined the team at RadioPublic and added two commits to address these two items:

• Fix the playlist margin funkiness that Gigi pointed out • Replace the CSS vars that Chris added during development

Like Chris mentioned, angular-cli currently doesn't expose a way to add the postcss-custom-properties plugin (although an issue was recently opened for that). I had considered making player.component.css an scss file in order to install Sass to use variables, but I decided to defer that since I'm not sure yet how you two feel about css preprocessors.

cqr commented 7 years ago

hey @pkarman been a while! I assume this assignment means it is up to me to push that big green button? any objections from @gcampo88 or @cavis?

cavis commented 7 years ago

No objections ... assuming that you've re-verified that thing @gcampo88 was seeing.

gcampo88 commented 7 years ago

does still seem to take away the rounded corners on the player when showing playlist! (left: dev, right: prod) image

but not sure how crucial that border-radius is down there on the playlist view...

binarynate commented 7 years ago

@gcampo88 , thanks for spotting that. I'm taking a look at a fix for the border-radius now.

gcampo88 commented 7 years ago

also, hi @BinaryNate! Congrats on joining RP!

binarynate commented 7 years ago

Thanks, @gcampo88 ! I just pushed a small tweak to bring back the bottom border-radius.

gcampo88 commented 7 years ago

Nice! Oops, just noticed this -- the builder offers a 500x500 "square" version of the player (which translates to it just being 500px wide and the usual height tall), and with this PR the background artwork extends down to 500px tall and looks bad. e.g.: image That said, we do have this ticket #153 to just get rid of that "square" option, since it's not actually a real option (we don't have a 500x500 version to offer them). So maybe we should just kill that now and give this the 👍 ? @cavis is out today -- perhaps he can weigh in early next week and then we can merge this thing!

binarynate commented 7 years ago

@gcampo88 , thank you for catching that. I just pushed a commit to fix it by adding the max-height of 200px like @cavis had suggested.

cavis commented 7 years ago

Yeah, let's not worry about the square view for now. (Other than not extending the BG-image into it, which seems to be taken care of now)