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

warn user if tick import errors #913

Closed
musoke opened this issue Jul 13, 2023 · 6 comments · Fixed by #969
Closed

warn user if tick import errors #913

musoke opened this issue Jul 13, 2023 · 6 comments · Fixed by #969
Assignees
Labels
Component: Log Book enhancement Improving existing functionality

Comments

@musoke
Copy link
Contributor

musoke commented Jul 13, 2023

Sometimes tick import has an error (e.g. due to #631 ). It is not obvious when this happens - I have been unsure whether I missed the success notification, it is still processing, or it failed.

Make it obvious. Briefly say what happened (maybe just say which tick failed). Dev builds could give more details.

Related: #891

How important is this to you (Please pick one)

  • Important
@musoke musoke added the enhancement Improving existing functionality label Jul 13, 2023
@clintonlunn
Copy link
Collaborator

clintonlunn commented Aug 21, 2023

i can probably take this. i added a toast.error that will alert when the tick import failed. i think there's still room to add more info.

in addition, i think this would be a good opportunity to transition the popup box from react/headlessui/ to radix-ui/alertdialog

@vnugent
Copy link
Contributor

vnugent commented Aug 22, 2023

@clintonlunn Thanks. Longer term I think we can let users provide their MP and/or theCrag profile URL in their profile settings once. They can then initiate a re-import any time wish without having to provide a profile URL each time.

@clintonlunn
Copy link
Collaborator

clintonlunn commented Aug 22, 2023

@vnugent I think that's a great suggestion. Do you have ideas of how/where we can store the mp/crag url?

clintonlunn added a commit that referenced this issue Aug 23, 2023
clintonlunn added a commit that referenced this issue Aug 23, 2023
@vnugent
Copy link
Contributor

vnugent commented Aug 23, 2023

Do you have ideas of how/where we can store the mp/crag url?

We can store it in our DB. The profile update endpoint will need an update.

@clintonlunn
Copy link
Collaborator

Do you have ideas of how/where we can store the mp/crag url?

We can store it in our DB. The profile update endpoint will need an update.

I'd be up for completing that work, just not very certain on where to start. But that can be an issue that gets created after this is closed.

@vnugent
Copy link
Contributor

vnugent commented Aug 26, 2023

We can introduce the profile settings in a separate PR. After this one is merged, I'd like to get PR #970 in (touching lots of files) so that contributors don't have to deal with merge conflicts.

vnugent pushed a commit that referenced this issue Aug 31, 2023
* feat: add progress and error indicator for importing mountainproject ticks #913
* feat: update importfrommtnproj component and add some basic tests #913
* feat: center spinner, better error handling on fetches #913
* feat: refactor component, remove unused isButton prop and logic from component and references in apps #913
* feat: disable button when loading #913
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Log Book enhancement Improving existing functionality
Projects
Development

Successfully merging a pull request may close this issue.

3 participants