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

Notification for Unallocated Attribute Points (fixes #4109) #4112

Merged
merged 3 commits into from Oct 8, 2014

Conversation

Projects
None yet
3 participants
@betaveros
Copy link
Contributor

commented Oct 4, 2014

Adds a notification when a player has unallocated attribute points. In addition to checking for unallocated attribute points, it checks the same condition used to determine whether to display the section on attribute points in the Stats & Achievements page.

To facilitate this, some of the logic conditions in the original notification code were refactored out into several controller methods in authCtrl.js. (I basically learned Angular yesterday, so I don't know if this arrangement makes sense, but it seems to work.)
unallocated-1
unallocated-2

betaveros added some commits Oct 4, 2014

DRY-refactor notification display conditions
* Condense the different notification icons into one span with a
  dynamically selected ng-class.
* Move logic for determining which notification icon to show and whether
  there are no notifications into AuthCtrl.
* Refactor out the two similar sequences of logic conditions into a
  helper function.
Add unallocated attribute points notification
* Uses i18n "haveUnallocated" string, to come in another commit
@betaveros

This comment has been minimized.

Copy link
Contributor Author

commented Oct 4, 2014

I have no idea what I'm doing and I don't know why the build is failing :(

@paglias

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2014

@betaveros it's not your fault. The tests on TravicCI are run with MongoDB 2.4 while we use 2.6 so it would fail in any case, don't worry.

The code seems fine, I can't test it right now but if you tested it and it's working than it's ok

paglias added a commit that referenced this pull request Oct 8, 2014

Merge pull request #4112 from betaveros/unallocated-notification
Notification for Unallocated Attribute Points (fixes #4109)

@paglias paglias merged commit 96b4144 into HabitRPG:develop Oct 8, 2014

1 check failed

continuous-integration/travis-ci The Travis CI build failed
Details
@paglias

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2014

Seems good to me, merging. @betaveros UUID for badge?

@betaveros

This comment has been minimized.

Copy link
Contributor Author

commented Oct 8, 2014

f4c81087-58db-4b55-a8b4-f8ee29f7e0bd

(I did get another pull request merged just 5 days ago :P)

@Alys

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2014

@betaveros : I'm giving you another contributor tier now.

If you provide your uuid in each pull request, it means that we don't have to keep track of it ourselves or spend time searching for it. There are several active contributors at the moment (too many to remember them all with complete reliability) and having each one paste their uuid into their own PRs is the most efficient way of handling this.

EDIT: level++ is done.

@betaveros betaveros deleted the betaveros:unallocated-notification branch Oct 14, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.