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

Markers customization #28

Closed
wants to merge 14 commits into from
Closed

Conversation

nrslt
Copy link
Contributor

@nrslt nrslt commented Dec 9, 2020

Hi guys,
I just wanted to quickly add the new markers to this existing PR.

  • blue one with tower icon = site marker
  • red one with Pyronear icon = alert marker
  • grey one with Pyronear icon = old fire marker

at this point it looks like this:
Screenshot 2020-12-09 at 12 20 51

To improve these, we will need to:

  • rethink clearly the way we want to display alerts + sites,
  • monitor the iconAnchor parameter so that the tip of the icon is correctly placed on the right coordinates (still unclear how this parameter behaves),
  • change the iconURL path

nrslt and others added 14 commits October 12, 2020 13:00
getting up to date with original repo
feat: Set up dockerized server for the dash app (#3)
…jects to be used in the homepage.py file ; changed the layout of the homepage in order to better fit our needs
feat: Sets the project for automatic deployment on Heroku (#5)
graphs.py new file + some changes in homepage.py layout ! (#7)
Added the slider and the callback for interactivity (#8)
getting latest commits and PRs
feat : Added new markers types and live alerts features (pyronear#23)
@Akilditu
Copy link
Collaborator

Akilditu commented Dec 9, 2020

  • iconURL path

Indeed this would be a good idea to host those icons somewhere else that in assets folder.
@frgfm maybe as we did for logos under pyronear.org ?

Copy link
Collaborator

@Akilditu Akilditu left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this PR !

Could you please resolve the 2 following flake8 errors :

./app/alerts.py:114:55: E261 at least two spaces before inline comment
./app/alerts.py:155:56: E261 at least two spaces before inline comment

@frgfm
Copy link
Member

frgfm commented Dec 9, 2020

@Akilditu yup, I think that would be the best thing to do 👌
We can also make a mock release on the website repo, and put all logos in the attachments! Whichever you think is best

@frgfm
Copy link
Member

frgfm commented Dec 9, 2020

@nrslt if the reason the PR was closed is only about updating the branch, you can merge master (or another branch for that matter) ;)
Would be better than reopening the same PR

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

Successfully merging this pull request may close these issues.

None yet

3 participants