-
-
Notifications
You must be signed in to change notification settings - Fork 61
Conversation
Screenshots of the change: https://imgur.com/a/C1w75Tx |
.vscode/settings.json
Outdated
"extensions": [".html", ".js", ".vue", ".jsx"] | ||
}, | ||
"eslint.run": "onSave", | ||
"eslint.autoFixOnSave": true |
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.
dont commit this file
om4r62Iuet7ByFWVF9OrZtcHOukk4WcZyQOxNWehTjZ87vdGSymqFeP5CIGncCKy | ||
03MGfXxKKvZa3kTcY6jW6DUJIm8QR7Ik9N8FN4bJcAmOWm6CgNoSyYVlW4TVjy/w | ||
mHC29LEtex2g8bRaQGl/dTnrHlOM1woV34KrpUU+0fzsJW7B+0IrRqtybgh4hktR | ||
-----END RSA PRIVATE KEY----- |
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.
I dont think this key is used anywhere in this code.
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.
I'm not sure what this key is either, maybe my SSH key?
@@ -0,0 +1 @@ | |||
ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQCi3kUTVjIVhkaa0GSr3vieErzdaVlZs1tski+XwGQZFFq2jCHfxHl361bLx6pQRwak/QQRZ6soPveHEvOxm/Ss7nMZS34twni3M709uAjanaFVx7hjiRjf+YLiAyyCWlsTt/1QPjMdSl3NUWiK5TUaR3qmOKI6utmqMwB2wT/5Py5wq/zSMcQ7BspytztkTug2o3eKvD7OhWvDfhtjhWgCybk1rETbVdxzbvsqqFsqqWX6YJII97IHlB5VoJBjm7TYopBS+amh4enFTwBkI8wzRuKdONoZ2sR138J1zMN9s4vEe9So1OInshbYM3JISCIeGtIZi5eaJP14N5ewj5K8Pi3i+4rP619f5ZUp4L9qbIKz6VGwmg4xEDc3ARYYFxpsTJz4+wLmmPadW7RqI528cwD/cfLmeO56pli7hviBT0itc9w2GFAhYFHm91EQBsTvkZTSWThXdx9Tt9vl3Cj1B9hpaHwaf8X99aln7NxMzmif6qr/46XwYGa4gVolBDX3fEbdbWXrUHpXMEwP24PpZBfq5QXVx8n9iEvaGEzbVUVwhTVcJrV3Ogt5C+X9AaUfu0ie+fiRW1grcfuk+R1VW2ZvZSbsIJeBhRP8n2bqBfrGF26IMJpkGGJEqyWZbbN+dApfeg4MUQaABbN8nYc9AkyqIFhs37Cj+hLzSqg3Lw== eugen.lefter@gmail.com |
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.
i think this key is also not used
src/components/MapPage/Map.js
Outdated
import OrganizationMarker from "./OrganizationMarker"; | ||
import { compose, lifecycle } from "recompose"; | ||
|
||
const styles = [ |
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 styles is a little big. You might want to break it off into its own file and import in
GoogleMapConfig.js
export const googleMapConfig = [...]
Map.js
import {googleMapConfig} from './GoogleMapConfig'
// ...
<GoogleMap
defaultOptions={googleMapConfig}
>
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.
agree
src/components/MapPage/Map.js
Outdated
<MarkerClusterer | ||
averageCenter={true} | ||
// icon={iconMarker} |
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.
remove unused commented code
src/index.js
Outdated
|
||
library.add(fab, fas); | ||
|
||
export const getRoutes = store => { | ||
return ( | ||
<div> | ||
<Switch> | ||
{/* <Route exact path="/" component={MyHomePageComponent} /> */} |
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.
remove unused commented code
If that is your SSH key, I'd recommend changing it right away. There are bots that scrape github for things like passwords, other credentials, and ssh keys, so they could use it to access your github or anything your ssh key is authenticated with. |
I don't have any review comments that haven't been left by others. When you say "give each institution a unique icon" do you mean each resource? Because there are currently around resources, 100, and as we grow there are gonna be a lot more. That'll be tough to sustain and onboard. |
Yes, each resource. |
Ok guys, I think I fixed all your comments and Im ready to merge this. Let me know If I need to do anything. For some reason I'm under impression that I need to close this pull request and create a new one. |
@Leftere It doesnt seem like you pushed your changes to this branch. Are you sure you added + committed all the right files? And that you pushed to the feat/map-update branch? |
bff5a96
to
8db195f
Compare
Issue #382
I have restyled the Google map to fit our design, I also got rid of lots of unnecessary default labels and icons to improve visual aspect.
Please let me know if you like it or if you have any other suggestions.
I also added a temporary new icon.
My plan is to give each institution a uniq icon so that the users knows what they click on.