Notification enhancements
-
@blakeyrat said in Notification enhancements:
Things that are not available to the user, and the user has no way to make the thing available on their own, should not show up on the screen.
<sarcasm> But.... it's a grid.... so you have to have something in those squares, it implies the feature should exist....
-
@blakeyrat said in Notification enhancements:
scary (like you're about to delete something)
why would thay be scary?
-
IMO just notify on the first like.
-
I would like to be notified only when some post have an unusual amount of upvotes, like what our badges system did back them.
I would also like to be notified when some post is near the deadline that allows for me to delete them.
-
Here's what I'm thinking for upvote notifications. Three options:
- All (this is what we have now)
- None (never see any upvote notification)
- 1st, 5th, 10th, 50th, 50xth
The site admin can set a default, then users can set it for themselves.
-
@boomzilla What about a 4th option for just the first like?
The current "Sam, Jeff and 281 others liked your post in..." would be my real preference if it stopped adding new notifications once I'd read it. Every like is rolled into a single notification per post until the notification is read, when it stops notifying for likes on that post.
I reckon that's fairly unique to just me though, so I also endorse the "first like only" option.
-
@loopback0 I gueeeeeessssssss I could put that in. Since you're so nice. Of course, none of this has been run by people at community.nodebb.org.
-
Commits are up (all correctly in feature branches this time!):
NodeBB - upvote-notifications branch
Persona Theme - upvote-notifications branch
Vanilla Theme - upvote-notifications branchPlease take a look / test if you are so inclined. From the user settings page:
-
Posted my intentions at NodeBB HQ:
-
Posted on community.nodebb.org:
One guy suggests a more logarithmic approach:
const log10 = num => Math.log(num)/Math.log(10); const isWhole = num => num % 1 === 0; const shouldNotify = upvotes => { // notify if [a power of 10] or if [5 * (a power of 10)] if (isWhole(log10(upvotes)) || isWhole(log10(upvotes / 5))) { return true; } return false; }
-
@boomzilla huh.... i thought nodebb was firmly es5 still....but pitaj suggests something in es6 syntax?
hmm.....
let's see that algorithm would notify for 5,10,50,100,500,1000, and so on notifications if i read it correctly?
that's a hell of a quick dropoff. and it misses the firstupvote as well.....
-
@boomzilla said in Notification enhancements:
Commits are up (all correctly in feature branches this time!):
NodeBB - upvote-notifications branch
Persona Theme - upvote-notifications branch
Vanilla Theme - upvote-notifications branch
Please take a look / test if you are so inclined.What? Why do I have to install a special version? Why not just put it online, so we can see it in action?
I'm sure no one will go on a tear-filled rant about user acceptance testing and software development practices...
-
@accalia said in Notification enhancements:
that's a hell of a quick dropoff.
Yeah, he calls it "future proof" but I think it's overkill. If someone starts running the next reddit on nodebb then they can submit a PR.
I guess he's also using that ES6 nonsense to avoid having to type
function
. Probably reason enough to reject it right there.
-
@boomzilla said in Notification enhancements:
Yeah, he calls it "future proof" but I think it's overkill.
if he wants to future proof it like that he can submit the PR to futureproof it if you ask me. :-P
@boomzilla said in Notification enhancements:
I guess he's also using that ES6 nonsense to avoid having to type function.
or he works with es6 enough that it's what comes to mind first. i know i had a hard time wen i submitted PRs to nodebb to remember doing the work in ES5
-
@accalia said in Notification enhancements:
or he works with es6 enough that it's what comes to mind first. i know i had a hard time wen i submitted PRs to nodebb to remember doing the work in ES5
What's the benefit of that style?
-
@boomzilla hipster points
-
@boomzilla said in Notification enhancements:
What's the benefit of that style?
Shorter and cleaner code, mostly
-
@RaceProUK said in Notification enhancements:
Shorter and cleaner code, mostly
const log10 = num => Math.log(num)/Math.log(10); function log10( num ){ return Math.log( num )/Math.log( 10 ); }
It wins on code golf points. I will fight you to the death for claiming it's cleaner, though.
-
@boomzilla It's more useful when you're using callbacks and Promises e.g.
anObject.aPromise(someArgs).then(result => doStuff(result));
vs
anObject.aPromise(someArgs).then(function (result) { return doStuff(result)); });
-
@RaceProUK said in Notification enhancements:
It's more useful when you're using callbacks and Promises e.g.
Yes, I can see the value there. But that's not how it's being over used here.
-
@boomzilla said in Notification enhancements:
What's the benefit of that style?
well for doing development on nodebb core, none. nodebb still supports nodejs 0.12 without harmony flags so it's es5.
as for es6 in general... well the fat arrow function form is more concise, it also doesn't mess with the
this
keywordfunction thistest() { const self = this; const doit = () => console.log(this === self); doit(); // Will print True to the console }
a full list of es6 feaures (not all are implemented in nodejs yet)
too see what features are supported where theres an app for that
-
Notifications, schmotifications. Until https://github.com/apapadimoulis/what-bugs/issues/81 is fixed and I can actually see my notifications, this thread is meaningless to me.
-
@accalia said in Notification enhancements:
and it misses the firstupvote as well.....
Math.log(1) == 0
-
@devjoe said in Notification enhancements:
is fixed and I can actually see my notifications, this thread is meaningless to me.
Well go make your own. The new thread button IS RIGHT THERE. /blakeyrat.
Err...sorry about that.
I think your issue probably happens when multiple notifications get merged but the number isn't updated properly. But that's kind of a WAG. But if that's true, then changing upvote notifications might effectively solve your problem.
-
@FrostCat said in Notification enhancements:
Just because they're big doesn't mean they're right.
-
Submitted the PR:
-
@boomzilla said in Notification enhancements:
Submitted the PR:
huh... build failed?
oh... looks like a missing dev dependencey
-
@accalia That's weird. Where do those go? The tests were working locally for me.
-
@boomzilla Just do an
npm install [whatever] --save-dev
and it'll put it in the right spot. Then commit that.
-
@boomzilla said in Notification enhancements:
@accalia That's weird. Where do those go? The tests were working locally for me.
possibly because you installed chai locally without adding it to the dependency list.
npm install --save-dev chai
will add it to the dependency list.you can also test if that's the case by deleting
node_modules/
and rerunning the tests after a plainnpm install
. if you have unmet dependencies the tests will start failing.
-
@Yamikuronue No, I mean...when npm installs the dev dependencies...where do they end up? I'm trying to figure out where it was that made it work locally without having it properly set up in dev dependencies. Which probably happened due to my git incompetence switching branches.
-
@boomzilla said in Notification enhancements:
when npm installs the dev dependencies...where do they end up?
In the packaqge.json:
If you have the dependency installed globally on your system (common with dependencies like chai), it wont' fail on your machine.
-
@accalia said in Notification enhancements:
you can also test if that's the case by deleting node_modules/ and rerunning the tests after a plain npm install. if you have unmet dependencies the tests will start failing.
What's weird is that I don't see it in
node_modules
like I'd expect.
-
@boomzilla Sounds like whatever package it is is installed globally for you
-
@Yamikuronue said in Notification enhancements:
If you have the dependency installed globally on your system (common with dependencies like chai), it wont' fail on your machine.
OK, maybe I did that...where are those things or how can I get a list of them?
-
@boomzilla said in Notification enhancements:
What's weird is that I don't see it in node_modules like I'd expect.
hmm... then you seem to have installed chai globally?
npm uninstall -g chai
?that will remove chai if it's installed globally
-
@boomzilla said in Notification enhancements:
where are those things or how can I get a list of them?
npm list -g
You probably want to toss in a
--depth=0
unless you care about the dependencies of your global packages
-
well that fixed the depenceny error..... why does Node 4.1 not like that PR?
-
@accalia Yeah...and it's the DB stuff that's failing there.
-
@boomzilla yeah. if i had permissions to i'd requeue the build....
i think this is one that's something strange that went spang into a corner on that particular build...
-
@Yamikuronue said in Notification enhancements:
npm list -g
You probably want to toss in a --depth=0 unless you care about the dependencies of your global packagesWow, their documentation is awful. Man page is identical. No clue that either of those would work as flags.
-
@boomzilla said in Notification enhancements:
Wow, their documentation is awful
Most OSS documentation is
-
@RaceProUK said in Notification enhancements:
Most OSS documentation is
Most commercial software documentation is too, especially when you want to do more than sit there and look at das blinkenlights.
-
@RaceProUK said in Notification enhancements:
@boomzilla said in Notification enhancements:
Wow, their documentation is awful
Most OSS documentation is
This is a special awfulness. Most OSS that has a man page will at least list the flags available.
-
@boomzilla To be fair, it did, just not the aliases.
-g
is short for--global
, which is listed (with a very confusing description).
-
@Yamikuronue said in Notification enhancements:
To be fair, it did, just not the aliases. -g is short for --global, which is listed (with a very confusing description).
Yes, the description is what's wrong. There's no indication that those are actual flags / options to pass. Labeling them 'configuration' makes it sound like you need to use a file or something. Or just use them right there...
$ npm ls global /home/boomzilla └── (empty) npm ERR! code 1
vs
$ npm list -g --depth=0 /usr/lib ├── grunt-cli@1.1.0 ├── istanbul@0.4.2 ├── mocha@2.4.5 ├── nodebb-plugin-imagemagick@1.0.1 -> /home/boomzilla/nodebb-plugin-imagemagick ├── nodebb-plugin-tdwtf-buttons@0.2.0 -> /home/boomzilla/nodebb-plugin-tdwtf-buttons ├── nodebb-plugin-youtube-lite@0.5.0 -> /home/boomzilla/nodebb-plugin-youtube-lite ├── nodebb-theme-persona@4.0.125 -> /home/boomzilla/nodebb-theme-persona ├── nodebb-theme-vanilla@5.0.68 -> /home/boomzilla/nodebb-theme-vanilla ├── npm@2.15.1 └── slush-nodebb-plugin@0.2.4
-
@boomzilla whaaaaa
why do you have NodeBB plugins installed globally?
-
@tufty said in Notification enhancements:
there's a stubborn little "1" stuck, as it were, on the end of my bell
WASH MOAR.
Also, most NodeBB weirdness (up to and including the inability to reply to any given post) goes away if you shift-reload the page.
-
@ben_lubar said in Notification enhancements:
why do you have NodeBB plugins installed globally?
That's just the plugins he's developing. He's got the source code in one set of folders, then uses
npm link
to "install" them to his NodeBB folder. When you do that, they look like global modules, but they're really not.
-
@ben_lubar said in Notification enhancements:
@boomzilla whaaaaa
why do you have NodeBB plugins installed globally?
I used
npm link
on them. So when npm goes to install them, it finds my git repo.