feat(breadcrumbs) experimental #1183

jeremysmartt · 2018-06-20T19:08:00Z

Description

First pass on breadcrumbs component and will be treated as experimental while we use it in multiple products to ensure quality.

closes #1053

Usage:

<td-breadcrumbs #breadcrumbs class="pad-left" separatorIcon="motorcycle">
    <a td-breadcrumb [routerLink]="'/'">Home</a>
    <a td-breadcrumb [routerLink]="'/layouts'">Layouts</a>
    <a td-breadcrumb [routerLink]="'/layouts2'">Layouts2</a>
    <a td-breadcrumb [routerLink]="'/layouts3'">Layouts3</a>
    <td-breadcrumb class="tc-grey-500">Manage List</td-breadcrumb>
</td-breadcrumbs>

What's included?

README.md
CovalentBreadcrumbsModule with TdBreadcrumbsComponent and TdBreadcrumbComponent
Usage on test bed
Added in experimental

Test Steps

npm run serve:test-bed
go to http://localhost:4200/
click on breadcrumbs link
See it working
Resize width of window to be smaller than width of breadcrumbs
See first breadcrumbs disappear for responsive
See hidden breadcrumbs count increase
See individual hidden breadcrumb width info

General Tests for Every PR

npm run serve:prod still works.
npm run tslint passes.
npm run stylelint passes.
npm test passes and code coverage is not lower.
npm run build:lib still works.

Screenshots or link to StackBlitz/Plunker

closes #1053

…style breadcrumbs

…b as the window shrinks

jotatoledo · 2018-07-11T10:31:40Z

src/platform/experimental/breadcrumbs/breadcrumbs.component.ts

+  constructor(private _elementRef: ElementRef, private _changeDetectorRef: ChangeDetectorRef) { }
+
+  ngAfterViewInit(): void {
+    this._resizeSubscription = merge(


Shouldnt this subscription be cleaned up on component destroy?

@jotatoledo You are correct, nice catch.

jotatoledo · 2018-07-11T17:31:55Z

src/platform/experimental/breadcrumbs/breadcrumbs.component.ts

+
+  ngDoCheck(): void {
+    if (this._elementRef && this._elementRef.nativeElement) {
+      this._widthSubject.next((<HTMLElement>this._elementRef.nativeElement).getBoundingClientRect().width);


What about hiding the argument of next behind a private getter?
Something like private get nativeElementWidth(): number {}

jotatoledo · 2018-07-11T17:38:25Z

src/platform/experimental/breadcrumbs/breadcrumbs.component.ts

+      // don't show the right chevron on the first breadcrumb
+      breadcrumbArray[0].displayIcon = false;
+    }
+    breadcrumbArray.map((breadcrumb: TdBreadcrumbComponent) => {


Wouldnt a forEach be better suited for iterating and mutating the elements in the array? considering that you arent really mapping the array elements into something new.

emoralesb05 · 2018-07-11T17:15:11Z

src/platform/experimental/breadcrumbs/breadcrumbs.component.spec.ts

+      ],
+      imports: [
+        NoopAnimationsModule,
+        RouterTestingModule.withRoutes([


Is there any reason to use mock routes?

just so the test doesn't fail with the routherlink in there

emoralesb05 · 2018-07-11T17:28:10Z

src/platform/experimental/breadcrumbs/breadcrumbs.component.ts

+    for (let i: number = this.hiddenBreadcrumbs.length; i < crumbsArray.length; i++) {
+      curTotCrumbWidth += crumbsArray[i].width;
+    }
+    let winWidth: number = this._elementRef.nativeElement.parentElement.offsetWidth - this._windowWidthPadding;


Should check the host here instead of the parent in case the component width changes.

emoralesb05 · 2018-07-11T17:30:06Z

src/platform/experimental/breadcrumbs/breadcrumbs.component.ts

+  private _widthSubject: Subject<number> = new Subject<number>();
+  private _resizing: boolean = false;
+  // Padding on the window when calculating the full size
+  private _windowWidthPadding: number = 20;


Is this needed if we check the host width rather than the parent width?

emoralesb05 · 2018-07-11T17:31:11Z

src/platform/experimental/breadcrumbs/README.md

+Example with all inputs/outputs: 
+
+```html
+<div layout-gt-sm="row" tdMediaToggle="gt-xs" [mediaClasses]="['push-sm']">


Lets remove this div and just keep the example with the component usage.

emoralesb05 · 2018-07-11T17:39:55Z

src/platform/experimental/breadcrumbs/README.md

+
+#### Methods
+
+ displayIcon: function(): boolean 


Not sure if we state that these properties are public. Since we use them internally to change the state of it.. which we should probably add a _ as a prefix, since that is the standard for internal properties.

jotatoledo · 2018-07-11T17:49:11Z

src/platform/experimental/breadcrumbs/breadcrumbs.component.ts

+   */
+  private setStyles(): void {
+    let breadcrumbArray: TdBreadcrumbComponent[] = this._breadcrumbs.toArray();
+    if (breadcrumbArray && breadcrumbArray.length > 0) {


Im almost sure that there is no need to check the existence of breadcrumbArray.
The not empty conditional could be shortened to if(breadcrumbArray.length)

jotatoledo · 2018-07-12T08:17:51Z

src/platform/experimental/breadcrumbs/breadcrumbs.component.ts

+  }
+
+  ngOnDestroy(): void {
+    if (this._resizeSubscription) {


Avoid the truthy check by declaring the sub variable with the initial value of Subscription.EMPTY;

Huh, i didnt know about that.

Showcase get/set methods as properties since thats how they are used

emoralesb05 · 2018-07-13T18:36:17Z

Just noticed that the icon is also clickable.. wonder if that will cause issues 🤔

this is done because the navigation or event shouldnt be triggered in that scenario

emoralesb05 · 2018-07-13T20:08:24Z

pushed changed to make sure clicking on the icon doesnt trigger navigation

* feat(breadcrumbs): Initial scaffolding for breadcrumbs * feat(breadcrumbs): Checkpoint checkin with breadcrumbs showing up * feat(breadcrumbs): code cleanup and adding graying out to last element * feat(breadcrumbs): Adding themes to experimental area. Using scss to style breadcrumbs * add to demo * feat(breadcrumbs): Responsive Breadcrumbs, removes the left-most crumb as the window shrinks * Adding sandbox area with routes to test-beds * Using ngAfterContentChecked lifecycle method * Adding title to demo * Update documentation Readme with usage * Moving tab-select into separate demo directory * Unit Tests for Breadcrumbs * Unit Tests for Breadcrumbs * chore(breadcrumbs): Update code from code review * fix(breadcrumb): Fix unit test * chore(breadcrumbs): Use Subscription.EMPTY to avoid needing null checks * chore(breadcrumbs): remove console.log * chore(): updates on README Showcase get/set methods as properties since thats how they are used * chore(breadcrumbs): remove media usage from breadcrumbs to reduce noise * fix(breadcrumbs): stop click event on the breadcrumb separator this is done because the navigation or event shouldnt be triggered in that scenario

feat(breadcrumbs): Initial scaffolding for breadcrumbs

60462cd

jeremysmartt changed the title ~~feat(breadcrumbs): Initial scaffolding for breadcrumbs~~ feat(breadcrumbs) Jun 20, 2018

jeremysmartt added in progress dont merge labels Jun 20, 2018

jeremy.smartt and others added 9 commits June 21, 2018 10:26

feat(breadcrumbs): Checkpoint checkin with breadcrumbs showing up

09b4339

feat(breadcrumbs): code cleanup and adding graying out to last element

f07235a

feat(breadcrumbs): Adding themes to experimental area. Using scss to …

6225fec

…style breadcrumbs

add to demo

00872d7

feat(breadcrumbs): Responsive Breadcrumbs, removes the left-most crum…

1f8d71a

…b as the window shrinks

Merge branch 'develop' into feature/breadcrumbs

99d4d43

Adding sandbox area with routes to test-beds

e1289b2

Using ngAfterContentChecked lifecycle method

2ff7bb2

Adding title to demo

e238436

jeremysmartt changed the title ~~feat(breadcrumbs)~~ feat(breadcrumbs) *experimental* Jul 10, 2018

jeremy.smartt added 5 commits July 10, 2018 14:42

Update documentation Readme with usage

993ab15

Merge branch 'develop' into feature/breadcrumbs

d00587d

Moving tab-select into separate demo directory

d1c0325

Unit Tests for Breadcrumbs

adccda6

Unit Tests for Breadcrumbs

9ce8512

jeremysmartt added ready for review and removed dont merge in progress labels Jul 11, 2018

jotatoledo reviewed Jul 11, 2018

View reviewed changes

emoralesb05 suggested changes Jul 11, 2018

View reviewed changes

jotatoledo reviewed Jul 11, 2018

View reviewed changes

jeremy.smartt added 3 commits July 11, 2018 16:54

chore(breadcrumbs): Update code from code review

45f2740

Merge branch 'develop' into feature/breadcrumbs

7d0ca24

fix(breadcrumb): Fix unit test

9bd7209

jotatoledo suggested changes Jul 12, 2018

View reviewed changes

jeremy.smartt and others added 4 commits July 12, 2018 10:02

chore(breadcrumbs): Use Subscription.EMPTY to avoid needing null checks

560d290

chore(breadcrumbs): remove console.log

95aef11

chore(): updates on README

29f1310

Showcase get/set methods as properties since thats how they are used

chore(breadcrumbs): remove media usage from breadcrumbs to reduce noise

e2c8e9d

emoralesb05 added the code lgtm label Jul 13, 2018

emoralesb05 approved these changes Jul 13, 2018

View reviewed changes

fix(breadcrumbs): stop click event on the breadcrumb separator

fd3799d

this is done because the navigation or event shouldnt be triggered in that scenario

emoralesb05 merged commit 256491e into develop Jul 13, 2018

emoralesb05 deleted the feature/breadcrumbs branch July 13, 2018 20:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(breadcrumbs) experimental #1183

feat(breadcrumbs) experimental #1183

jeremysmartt commented Jun 20, 2018 •

edited

Loading

jotatoledo Jul 11, 2018

jeremysmartt Jul 11, 2018

jotatoledo Jul 11, 2018

jotatoledo Jul 11, 2018

emoralesb05 Jul 11, 2018

jeremysmartt Jul 11, 2018

emoralesb05 Jul 11, 2018

emoralesb05 Jul 11, 2018

emoralesb05 Jul 11, 2018

emoralesb05 Jul 11, 2018

jotatoledo Jul 11, 2018

jotatoledo Jul 12, 2018

emoralesb05 Jul 12, 2018

jeremysmartt Jul 12, 2018

emoralesb05 commented Jul 13, 2018

emoralesb05 commented Jul 13, 2018

feat(breadcrumbs) *experimental* #1183

feat(breadcrumbs) *experimental* #1183

Conversation

jeremysmartt commented Jun 20, 2018 • edited Loading

Description

What's included?

Test Steps

General Tests for Every PR

Screenshots or link to StackBlitz/Plunker

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emoralesb05 commented Jul 13, 2018

emoralesb05 commented Jul 13, 2018

feat(breadcrumbs) experimental #1183

feat(breadcrumbs) experimental #1183

jeremysmartt commented Jun 20, 2018 •

edited

Loading