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.

Sign up for GitHub

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

Jump to bottom

Improve toast notification that shows on scheduled report creation #3558

Merged
merged 7 commits into from
Nov 28, 2023
Merged

Improve toast notification that shows on scheduled report creation #3558

merged 7 commits into from
Nov 28, 2023

Conversation

ericpgreen2
Copy link
Contributor

@ericpgreen2 ericpgreen2 commented Nov 27, 2023 •
edited
Loading

Addresses this UXQA item in Notion.

A couple notes:

  • The notifications/toasts code is quite hacky. At some point, we should review all our notifications, consider leveraging a third party library (like Melt UI), and refactor accordingly.
  • This PR also moves the scheduled reports code out of the features/dashboards directory to its own features/scheduled-reports directory

@ericpgreen2 ericpgreen2 self-assigned this Nov 27, 2023
@ericpgreen2 ericpgreen2 mentioned this pull request Nov 27, 2023
Closed
16 tasks
djbarnwal
djbarnwal approved these changes Nov 28, 2023
View reviewed changes
Copy link
Member

@djbarnwal djbarnwal 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!

web-common/src/components/icons/Check.svelte
>
<path
fill-rule="evenodd"
clip-rule="evenodd"
d="M18.5873 8.89285L10.4977 18.1532L5.40894 12.9881L7.01172 11.409L10.3986 14.8467L16.8928 7.4126L18.5873 8.89285Z"
d="M18.8161 6.57646C19.0184 6.75315 19.0449 7.05808 18.8763 7.26707L10.0888 18.1532L5.35091 13.3443C5.15711 13.1476 5.15947 12.831 5.35618 12.6372L6.24661 11.7599C6.44332 11.5661 6.7599 11.5685 6.9537 11.7652L9.98966 14.8467L17.1579 5.82284C17.3354 5.59943 17.6635 5.56958 17.8784 5.75728L18.8161 6.57646Z"
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Janet updated the Check icon. The new version is slightly larger (within its frame) and more closely mirrors the size of the Close icon, which is on the opposite side of the toast component.

@ericpgreen2 ericpgreen2 merged commit 5f29921 into main Nov 28, 2023
2 checks passed
@ericpgreen2 ericpgreen2 deleted the scheduled-report-created-toast branch November 28, 2023 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djbarnwal djbarnwal djbarnwal approved these changes

Assignees

@ericpgreen2 ericpgreen2

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ericpgreen2 @djbarnwal