ever-co / ever-gauzy

Ever® Gauzy™ - Open Business Management Platform (ERP/CRM/HRM/ATS/PM) - https://gauzy.co
https://gauzy.co
GNU Affero General Public License v3.0
2.3k stars 548 forks source link

Feature: Add notifications and loading indicators #73

Closed boyanstanchev closed 3 years ago

boyanstanchev commented 5 years ago

Add notifications and loading indicators everywhere in the application. There's already "angular2-toaster": "^7.0.0" in package.json.

evereq commented 5 years ago

@boyanstanchev did you add loading indicator to all pages? What about employees management page (http://localhost:4200/#/pages/employees)?

evereq commented 5 years ago

@AlexTasev please check every page and add loading indicators & toast notifications, same as we already did on many pages.

Note: toast notifications should always show context, e.g. record which was added/removed/ changed like "Organization 'Yo Technologies' was removed", etc.

chiragParmar93 commented 3 years ago

@evereq I have changed notifications for organization module and I have few questions for notification message, Right now in some modules showing success message like attached image(1) and some module showing message like image(2). so can you please confirm which message notification toast we need to add for whole app. image(1) image(2)

evereq commented 3 years ago

@parmarchirag93 how it is different in the code, I see we are using our own "toastrService" in some places? We not using it in other places? What we use there? Best if you can send links to both such places in code, so I review and decide which one we should use everywhere.

chiragParmar93 commented 3 years ago

Yes, we are using NbToastrService for toaster message and below is files there are using different toaster messages. For image(1) => gauzy\apps\gauzy\src\app\@shared\project-select\project\project.component.ts For image(2) => gauzy\apps\gauzy\src\app\@shared\expenses\recurring-expense-mutation\recurring-expense-mutation.component.ts

evereq commented 3 years ago

@parmarchirag93 let's use everywhere our own toastrService, i.e. "image(1)" and code like

this.toastrService.success(
                'NOTES.ORGANIZATIONS.EDIT_ORGANIZATIONS_PROJECTS.ADD_PROJECT',
                null,
                { name }
            );

It's better because it wraps translation inside, so we do NOT need to do it every time like:

    this.toastrService.primary(
                this.getTranslation(
                    'NOTES.ORGANIZATIONS.EDIT_ORGANIZATIONS_EXPENSE_CATEGORIES.ADD_EXPENSE_CATEGORY',
                    {
                        name: name
                    }
                ),
                this.getTranslation('TOASTR.TITLE.SUCCESS')
            );

So I think instead of above, we will need to just make it like:

this.toastrService.success(
                'NOTES.ORGANIZATIONS.EDIT_ORGANIZATIONS_EXPENSE_CATEGORIES.ADD_EXPENSE_CATEGORY',
                null,
                { name }
            );
evereq commented 3 years ago

@parmarchirag93 btw, I changed the implementation of the service itself to use "this.nbToastrService.primary" as I like blue color more than green for notifications (more consistent with Gauzy colors) - /gauzy/src/app/@core/services/toastr.service.ts#L23

Also, as part of this task, can you please verify why we have one method (danger) defined with async, while the other 2 methods without? https://github.com/ever-co/gauzy/blob/develop/apps/gauzy/src/app/@core/services/toastr.service.ts#L29 @rahul-rathore-576 if you have ideas, let us know. I think it should be all async or should all be without async. What do you think?

chiragParmar93 commented 3 years ago

via Hubstaff User: Chirag Parmar

Project: Gauzy Platform - https://app.hubstaff.com/projects/681079 Date Range: 12/22/20 - 12/22/20 Work session total: 3:39:28 Billable: No

Grand total: 3:39:28

rahul-rocket commented 3 years ago

@parmarchirag93 btw, I changed the implementation of the service itself to use "this.nbToastrService.primary" as I like blue color more than green for notifications (more consistent with Gauzy colors) - /gauzy/src/app/@core/services/toastr.service.ts#L23

Also, as part of this task, can you please verify why we have one method (danger) defined with async, while the other 2 methods without? https://github.com/ever-co/gauzy/blob/develop/apps/gauzy/src/app/@core/services/toastr.service.ts#L29 @rahul-rathore-576 if you have ideas, let us know. I think it should be all async or should all be without async. What do you think?

@evereq There is no need async in danger method. So we can remove it. @parmarchirag93

chiragParmar93 commented 3 years ago

@rahul-rathore-576 okay, I will remove async from danger method.

chiragParmar93 commented 3 years ago

via Hubstaff User: Chirag Parmar

Project: Gauzy Platform - https://app.hubstaff.com/projects/681079 Date Range: 12/23/20 - 12/23/20 Work session total: 3:45:52 Billable: No

Grand total: 7:25:20

chiragParmar93 commented 3 years ago

@evereq I have changed notifications for all modules. I have used ToastrService for all module. Please review it and give me your valuable feedback.

evereq commented 3 years ago

@parmarchirag93 amazing work, thank you!