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

fix: avoid empty breadcrumb at start when the for autogenerated case #56

Merged
merged 4 commits into from
Nov 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 2 additions & 29 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,36 +1,9 @@
## What is this about

## PR Checklist

Please check if your PR fulfils the following requirements:

- [ ] The commit message follows [the guidelines](https://github.com/scullyio/scully/blob/main/CONTRIBUTING.md#commit)
- [ ] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been added / updated (for bug fixes / features)

## PR Type

What kind of change does this PR introduce?

<!-- Please check the one that applies to this PR using "x". -->

- [ ] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, local variables)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Other... Please describe:

## What is the current behavior?

<!-- Please describe the current behavior that you are modifying, or link to a relevant issue. -->

Issue Number: N/A

## What is the new behavior?

## Does this PR introduce a breaking change?

- [ ] Yes
- [ ] No

<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below. -->

## Other information
2 changes: 2 additions & 0 deletions apps/got-demo-e2e/src/integration/app.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ describe('got-demo', () => {
cy.visit('/');
cy.contains('Books').click();
cy.get('xng-breadcrumb').contains('books');
// shouldn't have default seperator if Home breadcrumb is not defined
cy.get('.xng-breadcrumb-list').contains('/').should('not.exist');
});

it('should contain breadcrumbs for book with id', () => {
Expand Down
9 changes: 4 additions & 5 deletions libs/xng-breadcrumb/src/lib/breadcrumb.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import {
import { Router } from '@angular/router';
import { Observable, Subscription } from 'rxjs';
import { BreadcrumbItemDirective } from './breadcrumb-item.directive';
import { BreadcrumbService } from './breadcrumb.service';
import { Breadcrumb } from './types/breadcrumb';
import { BreadcrumbService, BreadcrumbDefinition } from './breadcrumb.service';

@Component({
selector: 'xng-breadcrumb',
Expand All @@ -21,8 +20,8 @@ import { Breadcrumb } from './types/breadcrumb';
})
export class BreadcrumbComponent implements OnInit, OnDestroy {
subscription: Subscription;
breadcrumbs: Breadcrumb[];
breadcrumbs$: Observable<Breadcrumb[]>;
breadcrumbs: BreadcrumbDefinition[];
breadcrumbs$: Observable<BreadcrumbDefinition[]>;
separatorTemplate: TemplateRef<void>;
private _separator = '/';

Expand Down Expand Up @@ -117,7 +116,7 @@ export class BreadcrumbComponent implements OnInit, OnDestroy {
this.subscription.unsubscribe();
}

handleRoute(breadcrumb: Breadcrumb) {
handleRoute(breadcrumb: BreadcrumbDefinition) {
const routeLink = breadcrumb.routeInterceptor
? breadcrumb.routeInterceptor(breadcrumb.routeLink, breadcrumb)
: breadcrumb.routeLink;
Expand Down
13 changes: 5 additions & 8 deletions libs/xng-breadcrumb/src/lib/breadcrumb.service.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Injectable } from '@angular/core';
import { ActivatedRoute, NavigationEnd, Router } from '@angular/router';
import { BehaviorSubject } from 'rxjs';
import { distinctUntilChanged, filter } from 'rxjs/operators';
import { filter } from 'rxjs/operators';
import { Breadcrumb } from './types/breadcrumb';
import {
BreadcrumbObject,
Expand All @@ -10,7 +10,7 @@ import {

type BreadcrumbConfig = BreadcrumbObject | BreadcrumbFunction | string;
type StoreMatcherKey = 'routeLink' | 'routeRegex' | 'alias';
type BreadcrumbDefinition = Breadcrumb & BreadcrumbObject;
export type BreadcrumbDefinition = Breadcrumb & BreadcrumbObject;
const PATH_PARAM = {
PREFIX: ':',
REGEX_IDENTIFIER: '/:[^/]+',
Expand Down Expand Up @@ -55,10 +55,7 @@ export class BreadcrumbService {
*/
private detectRouteChanges() {
this.router.events
.pipe(
filter((event) => event instanceof NavigationEnd),
distinctUntilChanged()
)
.pipe(filter((event) => event instanceof NavigationEnd))
.subscribe(() => {
this.previousBreadcrumbs = this.currentBreadcrumbs;
// breadcrumb label for base OR root path. Usually, this can be set as 'Home'
Expand All @@ -73,7 +70,7 @@ export class BreadcrumbService {
const rootBreadcrumb = this.extractObject(rootConfig?.data?.breadcrumb);
const storeItem = this.getFromStore(rootBreadcrumb.alias, '/');

if (rootBreadcrumb || storeItem) {
if (rootConfig || storeItem) {
return {
...storeItem,
...rootBreadcrumb,
Expand Down Expand Up @@ -304,7 +301,7 @@ export class BreadcrumbService {

/**
* Update the store to reuse for dynamic declarations
* If the store already has this route definition update it else add
* If the store already has this route definition update it, else add
*/
private updateStore(key: string, breadcrumb: BreadcrumbDefinition) {
const storeItemIndex = this.dynamicBreadcrumbStore.findIndex((item) => {
Expand Down