Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add applause button #124

Closed
wants to merge 9 commits into from
Closed

Add applause button #124

wants to merge 9 commits into from

Conversation

andrewda
Copy link

@andrewda andrewda commented Oct 16, 2018

  • I've checked that this isn't a duplicate pull request.

This adds an applause button to allow users to vote on their favorite swag!

Closes #104

image

Still TODO

  • Allow sorting

@zac-garby
Copy link
Collaborator

In my opinion the applause button is too big. It looks a bit out of place. I think maybe it should be the same size as the difficulty indicator circle (which could be enlarged slightly too)

@andrewda
Copy link
Author

@zac-garby Agreed - only difficulty is that the numbers start to overlap with the button at anything below 42px. I'll try bumping up the size of the difficulty indicator and we'll see if that makes it any better.

@plibither8
Copy link
Collaborator

This looks great!

There were two things that came to mind:

  1. The API URLs should not use the name of the company/organisation of the swag item. There can be instances where the same company has had multiple swag opportunities (for example, Digital Ocean). An ideal fix to this would be to add a timestamp along with the swag item name, and the timestamp would be of the date the opportunity was created or added to the list.
  2. Before pushing these changes, is there a way to set all claps back to 0?

@swapagarwal swapagarwal added the enhancement New feature or request label Oct 16, 2018
@swapagarwal
Copy link
Owner

swapagarwal commented Oct 16, 2018

Thanks @andrewda for picking this up! 🎉

A weird bug: Clap 10 items for an item and reload the page. The last clap isn't persisted. 🤔

On an unrelated note, should we restrict the user to one clap per item?

@plibither8
Copy link
Collaborator

👍 for limiting to one clap

@@ -14,6 +14,9 @@ link(rel="apple-touch-icon" size="128x128" href="/assets/img/logo.png")
link(rel="stylesheet" href="/assets/css/index.css")
link(rel="stylesheet" href="https://fonts.googleapis.com/css?family=Cabin")

link(rel="stylesheet" href="https://unpkg.com/applause-button/dist/applause-button.css")
Copy link
Contributor

Choose a reason for hiding this comment

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

imo Selectr is more important than applause so it should be loaded before applause

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this link & script have to be moved to the end of the body (link staying before the script tag).

<link rel="stylesheet"> is allowed in the body: https://html.spec.whatwg.org/multipage/links.html#body-ok

@aslafy-z
Copy link
Collaborator

@plibither8 @andrewda A solution (for the first point you rose) would be to use the actual reference url instead of devswag.io/:swagname. To avoid conflicts with others claps buttons we may want to prefix the url by a "devswag proxy": devswag.io/claps/:swagurl.

@plibither8
Copy link
Collaborator

Good idea @aslafy-z!

requestClaps(urls).then(resp => {
claps = resp;

const selectInput = document.getElementById('sorting');
Copy link
Contributor

Choose a reason for hiding this comment

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

You can delete this line since sortingInput is a global with the same value

@swapagarwal
Copy link
Owner

@andrewda Can you rebase this on master?

@swapagarwal
Copy link
Owner

Pinging @andrewda

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Applause-button to swag items
6 participants