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

Adding Warnings (Height-Graph/ Draw Text) #166

Open
hs7898753 opened this issue Apr 1, 2023 · 13 comments · May be fixed by #178
Open

Adding Warnings (Height-Graph/ Draw Text) #166

hs7898753 opened this issue Apr 1, 2023 · 13 comments · May be fixed by #178
Labels
bug Something isn't working

Comments

@hs7898753
Copy link
Contributor

Description

What is the problem you are facing

currently, height-graph feature is displaying an error message when it is used without a selected route.
Even some sort of compilation error comes when we use Draw Text.

What is your suggested solution

It should not display an compilation error message. Instead, it should display a warning message to select a route first.

Screenshots

Valhalla.FOSSGIS.Server.Demo.App.-.Brave.2023-04-01.11-32-32.mp4
@hs7898753 hs7898753 added the bug Something isn't working label Apr 1, 2023
@Sheikh-JamirAlam
Copy link
Contributor

Can I work on this issue?

@hs7898753
Copy link
Contributor Author

Yah sure. Wait for mentors to assign you the issue.

@nilsnolde
Copy link
Owner

Sure, please go ahead. Getting rid of errors is always good.

@Sheikh-JamirAlam
Copy link
Contributor

@hs7898753 I modifyed the code to make sure we don't see the error when user tries to open the height graph without mentioning the Waypoints. I used the react-semantic-toasts package to display the warning message, but I am facing this warning in the console and semantic-toast seems buggy.

Valhalla.FOSSGIS.Server.Demo.App.-.Google.Chrome.2023-04-02.21-25-49.mp4

Console warning :
image

Do you know any solution to this? Else I can use the Message element from semantic-ui-react package but that will not have the animations like this one.

@hs7898753
Copy link
Contributor Author

I think @nilsnolde can help you in better way.

@Sheikh-JamirAlam
Copy link
Contributor

@hs7898753 I used react-toastify and this is the output I got. I think it looks cool, do let me know about the warning message. react-toastify is better than react-semantic-toasts as it is not being maintained anymore. Do you think we should remove react-semantic-toasts comepletely and use react-toastify ?

Valhalla.FOSSGIS.Server.Demo.App.-.Google.Chrome.2023-04-02.22-42-23.mp4

@hs7898753
Copy link
Contributor Author

Yah! This looks good👏

@Sheikh-JamirAlam
Copy link
Contributor

@hs7898753 Can you tell me what's the function of this method in line 640. Its being called from this line.

@hs7898753
Copy link
Contributor Author

hs7898753 commented Apr 3, 2023

I think this is just to update the longitude-latitudes when the edit, drag, remove is triggered

@nilsnolde
Copy link
Owner

Do you think we should remove react-semantic-toasts comepletely and use react-toastify ?

Jep, I agree, it's better to move to the package that's maintained.

Can you tell me what's the function of this method in line 640. Its being called from this line.

On the right button panel you can draw polygons on the map which should be avoided by the route/isochrone. That's the function updating that polygon.

@Sheikh-JamirAlam
Copy link
Contributor

Jep, I agree, it's better to move to the package that's maintained.

Should I change the welcome toast and use react-toastify? And uninstall the react-semantic-toasts package?

@nilsnolde
Copy link
Owner

That'd be great!

In case you wanna do that, I'd suggest to raise another PR to do that first, as it's mostly a refactoring. Then we can merge it here and focus on the functional changes. How does that sound?

@Sheikh-JamirAlam
Copy link
Contributor

Sure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants