Skip to content
This repository has been archived by the owner on Oct 15, 2022. It is now read-only.

BBC: Use image sizes closer to the display sizes, make signal high #667

Closed
wants to merge 7 commits into from
Closed

BBC: Use image sizes closer to the display sizes, make signal high #667

wants to merge 7 commits into from

Conversation

TomBebbington
Copy link
Contributor

This makes the detailed episode image bigger and the tiled episode smaller, and gives the spice a high signal because the triggers are very specific.

Old sizes

Desktop view

Desktop view

Mobile view

Wouldn't load...

Updated sizes

Desktop view

Desktop view

Mobile view

Mobile view

@jagtalon
Copy link
Member

jagtalon commented May 5, 2014

@TopHattedCoder Cool! @chrismorast What do you think about this?

@@ -45,6 +45,7 @@
Spice.add({
id: 'bbc',
name: 'BBC Shows',
signal: 'high',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TopHattedCoder I haven't seen the key signal before. What does it do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bradcater There are multiple IAs appearing at the same time. Setting signal to high will let us know which ones should show up first. :)

@jagtalon
Copy link
Member

jagtalon commented May 5, 2014

Hey, @TopHattedCoder. Here's the feedback from @chrismorast:

These CSS changes should be good (make sure to prepend .zci--bbc to localize the changes to the instant answer):

screen shot 2014-05-05 at 8 26 20 am

The detail view should also display the time. It should be style exactly like the recipes.

So:

  • Title of Show
  • Time Slot | Running Time (styled as a subtitle)
  • [Space]
  • Snippet

screen shot 2014-05-05 at 8 34 32 am

@russellholt
Copy link
Contributor

I think we need to use the products_simple template group for this. Recipes' detail view is not a good example to copy.

@jagtalon
Copy link
Member

jagtalon commented May 9, 2014

There were some changes to the way we handle tiles which is responsible for the merge conflict. Check out #680 for more details. :)

@TomBebbington
Copy link
Contributor Author

Okay, I'll rebase the commits to be after the tile changes and see if that
fixes anything, then use the product template and add more details to the
programme summary. Thanks for the update!

@jagtalon
Copy link
Member

@TomBebbington Sorry about the hassle, but we made a bunch of final changes before the launch last week. Is it okay if you merge master into your branch?

@TomBebbington
Copy link
Contributor Author

Yeah, sure.

On Fri, May 30, 2014 at 6:13 AM, Jag Talon notifications@github.com wrote:

@TomBebbington https://github.com/TomBebbington Sorry about the hassle,
but we made a bunch of final changes before the launch last week. Is it
okay if you merge master into your branch?


Reply to this email directly or view it on GitHub
#667 (comment)
.

@yegg yegg closed this May 30, 2014
@@ -120,24 +110,8 @@


// Find the programme image and return it
function image(item) {
return "http://ichef.bbci.co.uk/images/ic/272x153/" + (item.programme.image ? item.programme.image.pid : "legacy/episode/"+item.programme.pid) + ".jpg";
function image(item, big) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I like that we're choosing between sizes here.

@jagtalon
Copy link
Member

@TomBebbington Sorry about closing this! We didn't see that there were PRs referencing the bttf branch.

};
},
templates: {
item: 'basic_image_item',
detail: Spice.bbc.detail,
group: 'products_simple',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was renamed to the media template, but this still works.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants