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

feat: add button to complete missing URL and legal information #2446

Merged
merged 1 commit into from Jan 5, 2024

Conversation

mstykow
Copy link
Collaborator

@mstykow mstykow commented Dec 21, 2023

Summary of changes

  • add magic wand button in URL input field
  • show success or error toasts when user clicks magic wand button
  • also show toasts on copy/paste clipboard buttons
  • fix tryit function to wait for promise
  • throw for error status by default to avoid needing to perform response.ok checks

image

image

Context and reason for change

Closes #2445.

How can the changes be tested

Select a signal with only partial information and try to fill the missing information with the new button. Also note that the button should disappear after the attempt, whether successful or not.

@vasily-pozdnyakov
Copy link
Contributor

  1. Sometimes the button does not disappear after the first click but after the second.
  2. Disappearing button is quite confusing for the user (especially when it comes back after some actions). Instead of hiding the button I would propose to use a toast with the relevant info. Or maybe consider some other alternatives.

@mstykow mstykow force-pushed the feat-button-to-complete-missing-url-legal branch from 4a7db91 to 6348d48 Compare December 22, 2023 17:01
@mstykow
Copy link
Collaborator Author

mstykow commented Dec 22, 2023

  1. Sometimes the button does not disappear after the first click but after the second.
  2. Disappearing button is quite confusing for the user (especially when it comes back after some actions). Instead of hiding the button I would propose to use a toast with the relevant info. Or maybe consider some other alternatives.

I'm using a toaster now so that the hiding is no longer necessary.

@antonbauhofer
Copy link
Member

First of all, this feature is really awesome!
I'm not sure if the usage is quite as expected, though:

  • Sometimes I get the toast "Added information where possible" even though seemingly nothing was added to the attribution.
  • Maybe we should communicate the requirements for the icon to work in some way. E.g. that it requires a package type (it does, right?). I could imagine pointing that out in the red toast or in the tooltip of the icon.

@mstykow mstykow force-pushed the feat-button-to-complete-missing-url-legal branch 2 times, most recently from 0af838c to bc737c2 Compare January 5, 2024 11:08
@mstykow
Copy link
Collaborator Author

mstykow commented Jan 5, 2024

First of all, this feature is really awesome! I'm not sure if the usage is quite as expected, though:

  • Sometimes I get the toast "Added information where possible" even though seemingly nothing was added to the attribution.
  • Maybe we should communicate the requirements for the icon to work in some way. E.g. that it requires a package type (it does, right?). I could imagine pointing that out in the red toast or in the tooltip of the icon.

I've improved the error message for when we cannot fetch information because the package type is unknown.
I've also added a check to see if more information was found. If not, there is now a special message for this case:
image

Copy link
Member

@antonbauhofer antonbauhofer left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@mstykow mstykow force-pushed the feat-button-to-complete-missing-url-legal branch from bc737c2 to 2696b2f Compare January 5, 2024 15:17
@mstykow mstykow enabled auto-merge January 5, 2024 15:18
- add magic wand button in URL input field
- show success or error toasts when user clicks magic wand button
- also show toasts on copy/paste clipboard buttons
- fix tryit function to wait for promise
- throw for error status by default to avoid needing to perform `response.ok` checks

closes #2445

Signed-off-by: Maxim Stykow <maxim.stykow@tngtech.com>
@mstykow mstykow force-pushed the feat-button-to-complete-missing-url-legal branch from 2696b2f to 513a4b2 Compare January 5, 2024 15:26
@mstykow mstykow merged commit d1a855d into main Jan 5, 2024
5 checks passed
@mstykow mstykow deleted the feat-button-to-complete-missing-url-legal branch January 5, 2024 15:35
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.

Add button to complete missing URL and legal information
3 participants