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

Fixes#986 : Limits the Time for showing Generated Badge Link #1005

Merged
merged 1 commit into from Jun 18, 2018

Conversation

4 participants
@ParthS007
Copy link
Member

ParthS007 commented Jun 14, 2018

Fixes #986

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream development branch.
  • I have added necessary documentation (if appropriate)

Preview Link

Changes proposed in this pull request:

  • Limits the Time for showing Generated Badge Link i.e 10 seconds.
  • Now, Link of previously created badges is not visible on revisiting the page or after the new login.

screenshot from 2018-06-14 18-10-30

@gabru-md
Copy link
Member

gabru-md left a comment

lgtm

@open-event-bot open-event-bot bot removed the needs-review label Jun 14, 2018

@ParthS007

This comment has been minimized.

Copy link
Member

ParthS007 commented Jun 15, 2018

@djmgit Please review and Merge. Thanks 😃

sendBadge(badgeData) {
const _this = this;
let badgeRecord = _this.get('store').createRecord('badge', badgeData);
badgeRecord.save()
.then(record => {
_this.set('badgeGenerated', true);
_this.set('genBadge', record);
_this.get('notify').success('Badge generated Successfully');
var notify = _this.get('notify');

This comment has been minimized.

@djmgit

djmgit Jun 16, 2018

Member

@ParthS007 so, after the badges are generated, this notification will appear for some time. So suppose the user is generating a lot of badges (has given a large csv file) and goes away, comes back after some time and within that time the notification has already disappeared. How will the user download the badges then?

This comment has been minimized.

@gabru-md

gabru-md Jun 16, 2018

Member

The my-badges section will always have the links to download them @djmgit

This comment has been minimized.

@djmgit

djmgit Jun 16, 2018

Member

That is true, but the user will have to go to my-badges section for downloading incase he misses the notification. Thats becoming a sloppy UX, isn't it?

@@ -118,18 +118,11 @@
</div>
</div>
<button {{action "submitForm"}} class="ui fluid button">Create Badges</button>
{{#if badgeGenerated}}

This comment has been minimized.

@djmgit

djmgit Jun 16, 2018

Member

Instead of using timed notification, can't we go with this approach only? Can this variable (badgeGenerated) be set to false whenever the page od reloaded?

This comment has been minimized.

@ParthS007

ParthS007 Jun 17, 2018

Member

@djmgit You are correct but this app is behaving like single page application and page not get refreshed on coming back again to create badges route. We have to override route constructor for setting up the variable false on route reload. I was following the same approach but unable to find the workaround for overriding the route constructor that's why I have done this way. For now, we can go with this. As soon as we find the way we can change the method.
What's your say @djmgit @yashLadha @gabru-md?

@djmgit
Copy link
Member

djmgit left a comment

Please have look at the online comments.

@djmgit

djmgit approved these changes Jun 18, 2018

Copy link
Member

djmgit left a comment

@ParthS007 please resolve the conflicts. We have to definitely iterate on this later. As of now I will merge it.

@ParthS007 ParthS007 force-pushed the ParthS007:link branch from aa2a093 to ba29cea Jun 18, 2018

@ParthS007 ParthS007 force-pushed the ParthS007:link branch from d751eb0 to f8afaee Jun 18, 2018

@open-event-bot

This comment has been minimized.

Copy link

open-event-bot bot commented Jun 18, 2018

Hi @ParthS007!

It looks like one or more of your builds have failed.
I've added the relevant info below to save you some time.

@djmgit djmgit merged commit d0f3551 into fossasia:development Jun 18, 2018

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ParthS007 ParthS007 deleted the ParthS007:link branch Jun 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment