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

Review all toast notifications #81

Merged
merged 4 commits into from
May 10, 2023
Merged

Conversation

bcgv123
Copy link
Contributor

@bcgv123 bcgv123 commented May 5, 2023

https://apps.nrs.gov.bc.ca/int/jira/browse/SHOWCASE-2960

New common functions have been added to 'useToast.ts' - success, error and info. These three new functions contain a new (appropriate) title format (e.g. verb-ing object 'Configuring object'), and message defined by our UI/UX designer. Also, timeouts are being read from the constant file.

SUCCESS:

Screenshot 2023-05-05 at 1 59 20 PM

ERROR:

Screenshot 2023-05-05 at 1 59 32 PM

INFO:

Screenshot 2023-05-09 at 10 58 56 AM

Description

Types of changes

Checklist

  • I have read the CONTRIBUTING doc
  • I have checked that unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@github-actions
Copy link

github-actions bot commented May 5, 2023

Coverage Report (Application)

Totals Coverage
Statements: 75% ( 51 / 68 )
Methods: 62.5% ( 5 / 8 )
Lines: 82.61% ( 38 / 46 )
Branches: 57.14% ( 8 / 14 )

@bcgv123 bcgv123 marked this pull request as draft May 5, 2023 20:01
@github-actions
Copy link

github-actions bot commented May 5, 2023

Coverage Report (Frontend)

Totals Coverage
Statements: 38.15% ( 335 / 878 )
Methods: 34.67% ( 69 / 199 )
Lines: 45.94% ( 243 / 529 )
Branches: 15.33% ( 23 / 150 )

@bcgv123 bcgv123 marked this pull request as ready for review May 5, 2023 21:43
Copy link
Member

@jujaga jujaga left a comment

Choose a reason for hiding this comment

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

Some general styling and consistency fixes needed. There are some jarring presentation/wording quirks with present-progressive and a past participle statements in one toast. Some of these messages suffice just being one liner alerts instead of being verbose toasts with a title and body. Would recommend consulting with Tyler on this if this hasn't been raised yet and how to simplify messaging so that it remains concise, but also consistent with the rest of the other alert styles.

frontend/src/App.vue Outdated Show resolved Hide resolved
frontend/src/components/form/CopyToClipboard.vue Outdated Show resolved Hide resolved
frontend/src/components/object/DeleteObjectButton.vue Outdated Show resolved Hide resolved
frontend/src/components/object/ObjectFileDetails.vue Outdated Show resolved Hide resolved
frontend/src/components/object/ObjectList.vue Outdated Show resolved Hide resolved
frontend/src/components/object/ObjectUpload.vue Outdated Show resolved Hide resolved
frontend/src/components/object/ObjectUpload.vue Outdated Show resolved Hide resolved
frontend/src/lib/primevue/useToast.ts Outdated Show resolved Hide resolved
frontend/src/lib/primevue/useToast.ts Show resolved Hide resolved
frontend/src/utils/constants.ts Outdated Show resolved Hide resolved
@bcgv123 bcgv123 force-pushed the 2960-review-all-toast-notifications branch from 3d02334 to e1bdc5e Compare May 9, 2023 16:07
@bcgv123 bcgv123 requested review from jujaga and kamorel May 9, 2023 18:16
@jujaga jujaga merged commit 47896aa into master May 10, 2023
@jujaga jujaga deleted the 2960-review-all-toast-notifications branch May 10, 2023 16:36
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.

4 participants