Skip to content

Star rating#159

Open
SavageWilliam wants to merge 6 commits into
masterfrom
star-rating
Open

Star rating#159
SavageWilliam wants to merge 6 commits into
masterfrom
star-rating

Conversation

@SavageWilliam

@SavageWilliam SavageWilliam commented Feb 15, 2017

Copy link
Copy Markdown
Contributor

Satisfies #150 & #158
Uses sprites to display star rating

image

Works towards #86 - not complete

@Jbarget

Jbarget commented Feb 16, 2017

Copy link
Copy Markdown
Member

hey @SavageWilliam, seen this but wont have time to merge until next week probs cos im working/getting ready to fly out to israel n all that comes with it

hope thats ok!

@Jbarget Jbarget left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

a few minor things

Comment thread public/main.css
}

.navbar__listitem {
list-style-type: none;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

2 space indentation here please :)

Comment thread views/resources.hbs
<div class='resources'>
{{#each resources}}
<section class="resource">
<h3 class="resource_title"><a href={{this.url}}>{{this.title}}</a></h3>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

a few empty lines here, please remove

Comment thread views/resources.hbs
{{/if}}
</p>

{{# if ../credentials}}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

im just curious: what is this line saying?

@Jbarget

Jbarget commented May 16, 2017

Copy link
Copy Markdown
Member

@SavageWilliam just going through my PRs. just a reminder

@Jbarget

Jbarget commented May 26, 2017

Copy link
Copy Markdown
Member

removing myself from assignment, assign me again if you want this merged in :)

@Jbarget Jbarget removed their assignment May 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants