-
Notifications
You must be signed in to change notification settings - Fork 77
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
WIP: PLpeople popups #422
WIP: PLpeople popups #422
Conversation
OK! I've merged LBLD upstream and republished as publiclab/leaflet-blurred-location-display#102 and publiclab/leaflet-blurred-location-display#101 If we resolve conflicts here, are we ready to merge this? Thanks! Hope you're well! |
@@ -0,0 +1,57 @@ | |||
TimeAgo = function TimeAgo() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really cool. Did you write this? You should consider releasing it as a stand-alone NPM module. It'd be useful and being maintainer on a small NPM module is a pretty interesting experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jywarren I didn't, it is from here: https://gist.github.com/caiotarifa/30ae974f2293c761f3139dd194abd9e5 but I had to modify it for what we needed!
I also used the modified version here: https://github.com/publiclab/plots2/blob/668c28e5822f17438b7b4da8d39e27d3af7f38a1/app/assets/javascripts/timeago.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome. Would you have any time to try rebasing this to get it merged? I don't think it should be too tough... but i understand if you don't have time right now! Be safe and well!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting that TimeAgo is not appearing on publiclab.org, i think we need to add a JS include there somewhere... https://publiclab.org/map#11/40.67759191045513/-73.87728530215101
https://github.com/publiclab/plots2/pull/7715/files#diff-a9c3bd311eab80c9ebe6a69830f9ad02R62 seems to include it though... hmm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to clog up this thread. I think it's a load-order issue and i'll re-order it. All good!
@jywarren NPM is showing blurred-location-display still at 1.1.0, can you publish the newest version? https://www.npmjs.com/package/leaflet.blurred-location-display |
Ah yes, sorry! |
Hi @sagarpreet-chadha -- actually i'm not a collaborator on LBLD - can you add me? Thank you! |
Hey Jeff, added you as maintainer 👍 |
OK! Thank you @sagarpreet-chadha !!! |
Great thanks!! I'll get this done. |
Solved in #453! |
Fixes #421 (<=== Add issue number here)
This depends entirely on publiclab/leaflet-blurred-location-display#96 and cannot be merged until that PR is merged and published in LBLD.
The "Joined [time ago]" depends on new data added to the API in plots2: publiclab/plots2#7562
This is what the new popups look like:
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
@publiclab/reviewers
for help, in a comment belowIf tests do fail, click on the red
X
to learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Thanks!