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

Show people in a region as a bulleted list within the popup #136

Merged
merged 4 commits into from
Aug 17, 2021

Conversation

daemon1024
Copy link
Member

Fixes #133

@welcome
Copy link

welcome bot commented Aug 17, 2021

Thanks for opening this pull request! Dangerbot will test out your code and reply in a bit with some pointers and requests.
There may be some errors, but don't worry! We're here to help! 👍🎉😄

@daemon1024
Copy link
Member Author

image
It works 😄

@daemon1024
Copy link
Member Author

/cc @jywarren

@daemon1024
Copy link
Member Author

//offtopic
Btw just noticed, we could use some updates on welcome bot messages. Potential good first issues, across repositories.

@jywarren
Copy link
Member

//offtopic
Btw just noticed, we could use some updates on welcome bot messages. Potential good first issues, across repositories.

great idea.

OK, and this one is good to merge?

@jywarren
Copy link
Member

Looks like it! Thanks!!!

@jywarren
Copy link
Member

Would you like to add a package.json version increment of 0.0.1?

@jywarren
Copy link
Member

and run npm install so we get an updated package-lock.json too? Then I'll be able to publish it as a new version on NPM

@daemon1024
Copy link
Member Author

Done :)

@jywarren jywarren merged commit 86e8739 into publiclab:main Aug 17, 2021
@welcome
Copy link

welcome bot commented Aug 17, 2021

Congrats on merging your first pull request! 🙌🎉⚡️
Your code will likely be published to https://publiclab.org in the next few days.
In the meantime, can you tell us your Twitter handle so we can thank you properly?
Now that you've completed this, you can help someone else take their first step!
See: Public Lab's coding community!

@jywarren
Copy link
Member

As soon as this publishes, let's link and follow to the PR in LEL, and then same to plots2 -- sound good? Thank you!!! Also doesn't have to be tonight, it's so late!

@daemon1024
Copy link
Member Author

Yup, Sounds good to me. Thank You and Good Night 🌃

@jywarren
Copy link
Member

Published! Good night!!!

@jywarren
Copy link
Member

If dependabot doesn't open it, we can open it manually!

@jywarren
Copy link
Member

@jywarren
Copy link
Member

OK! We more or less got it... still seeing an "undefined" but very close!!! Did we use the wrong property name?

image

publiclab/plots2#10050

@jywarren
Copy link
Member

Hmm.

 {
      "doc_id": 768638,
      "doc_type": "PLACES",
      "doc_url": "/profile/charliejohnson",
      "doc_title": "charliejohnson",
      "doc_score": null,
      "doc_author": null,
      "doc_image_url": "https://www.gravatar.com/avatar/6e62cf8e8246c20b6b9839c175dc996a",
      "latitude": "0",
      "longitude": "0",
      "blurred": true,
      "category": "PLACES",
      "place_name": null,
      "created_at": "2021-08-10T11:07:07.000Z",
      "time_since": null,
      "comment_count": null
    }

@@ -151,6 +151,7 @@ BlurredLocationDisplay = function BlurredLocationDisplay(options) {

var id = obj["id"] ;
var url = obj["url"] ;
var title = obj["title"] ;
Copy link
Member

Choose a reason for hiding this comment

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

Here, it should work, i think:

obj["title"] = data.items[i].doc_title ;

@daemon1024
Copy link
Member Author

The url is right, just the title is undefined somehow 🤔

@jywarren
Copy link
Member

For the screenshot above, I was def. getting doc_title values...

{
  "doc_id": 766513,
  "doc_type": "PLACES",
  "doc_url": "/profile/dennlawgroup",
  "doc_title": "dennlawgroup",
  "doc_score": null,
  "doc_author": null,
  "doc_image_url": "/public/system/users/photos/000/766/513/thumb/denn-law-logo-1x-2.png",
  "latitude": "42",
  "longitude": "-71",
  "blurred": true,
  "category": "PLACES",
  "place_name": null,
  "created_at": "2021-07-21T08:33:27.000Z",
  "time_since": null,
  "comment_count": null
}

@jywarren
Copy link
Member

@jywarren
Copy link
Member

We override the JSONparser function in LEL!

@daemon1024
Copy link
Member Author

Ohhh.

@daemon1024
Copy link
Member Author

So just one more release then :D Making a PR asap.

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

Successfully merging this pull request may close these issues.

Show people in a region as a bulleted list within the popup
2 participants