feat(shared): #1632 create shared breadcrumb component

DashboardHub / PipelineDashboard

Dashboard for your Deployment pipeline https://dashboardhub.io/

https://pipeline.dashboardhub.io/

GNU General Public License v3.0

152 stars 114 forks source link

feat(shared): #1632 create shared breadcrumb component #1639

Closed webkhushboo closed 4 years ago

webkhushboo commented 4 years ago

closes #1632 #1633

Notes

A summary of what was achieved in this PR

[ ] Created shared breadcrumb component
[ ] Added shared breadcrumb component inside monitor list component
[ ] Added shared breadcrumb component inside token list component
[ ] Added shared breadcrumb component inside rating component
[ ] Added shared breadcrumb component inside ping list component

eddiejaoude commented 4 years ago

Looks good. I left a few small comments.

Some thoughts:

Should this be in the new component this.breakpointObserver, instead of in multiple the parents?
I just realised when testing that create/edit monitor and token pages should have breadcrumb also
Project view page should use the breadcrumb too

webkhushboo commented 4 years ago

Looks good. I left a few small comments.

Some thoughts:

Should this be in the new component this.breakpointObserver, instead of in multiple the parents?

I just realised when testing that create/edit monitor and token pages should have breadcrumb also

Project view page should use the breadcrumb too

While I was coding , I was also thinking to move this.breakpointObserver inside breadcrumb component but it was also used for table columns in the parent components so I didn't touched it. I will add the this.breakpointObserver inside breadcrumb for isSmallScreen check.Let me know your thoughts on it. I will add the breadcrumb in the create/edit monitor and tokens pages and view component too.

Question: In view component we have menu for showing edit and delete actions for project in breadcrumb. Do we need to show this menu on all pages or just view component page ?

eddiejaoude commented 4 years ago

While I was coding , I was also thinking to move this.breakpointObserver inside breadcrumb component but it was also used for table columns in the parent components so I didn't touched it. I will add the this.breakpointObserver inside breadcrumb for isSmallScreen check.Let me know your thoughts on it.

Ah ok, if it is used for something else, then best to leave it where it is 👍

I will add the breadcrumb in the create/edit monitor and tokens pages and view component too.

Thank you 🤓

In view component we have menu for showing edit and delete actions for project in breadcrumb. Do we need to show this menu on all pages or just view component page ?

Good question! I think the easiest for now is to leave it on all page

eddiejaoude commented 4 years ago

Looks good. I left 1 inline comment.

I notice that the project logo/placeholder looks fine on the dashboard and tokens page, but on the monitors page it still has the Dashboardhub placeholder (screenshots below)

Dashboard page

Tokens page

Monitors page

webkhushboo commented 4 years ago

Looks good. I left 1 inline comment.

I notice that the project logo/placeholder looks fine on the dashboard and tokens page, but on the monitors page it still has the Dashboardhub placeholder (screenshots below)

Yes you are right , we are showing fallback icon as dashboard hub logo in monitor list component. I have removed it now.

Dashboard page

Tokens page

Monitors page