Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Fix repeater collapsible on push #1724

Merged
merged 29 commits into from
Jun 13, 2018
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
3f4222e
Resolving Card action bar still visable after hiding with *ngIf #1421
blackbaud-conorwright Mar 7, 2018
a450054
Addressed PR style comments
blackbaud-conorwright Mar 8, 2018
4b809aa
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright Mar 9, 2018
d55f351
Merge branch 'master' into master
Blackbaud-SteveBrush Mar 9, 2018
833bcc7
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright Mar 12, 2018
b553659
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright Mar 15, 2018
ab82ba3
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright Mar 19, 2018
db21c76
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright Mar 27, 2018
a282a86
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright Mar 30, 2018
d210c2b
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright Apr 10, 2018
35e789d
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright Apr 17, 2018
636c5dd
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright Apr 23, 2018
bdc97a6
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright May 1, 2018
6104aaf
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright May 2, 2018
37f50ec
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright May 8, 2018
76279cd
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright May 14, 2018
2324e90
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright May 16, 2018
d48b8f2
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright May 23, 2018
b2e318a
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright May 25, 2018
8d52945
Merge remote-tracking branch 'upstream/master'
blackbaud-conorwright May 25, 2018
119bcb2
made isCollapsible into an observable so that it pushes on updates
blackbaud-conorwright May 29, 2018
df8f3fc
removed unused import
blackbaud-conorwright May 29, 2018
43ee008
Merge branch 'master' into fix-repeater-collapsible-on-push
Blackbaud-SteveBrush Jun 4, 2018
9cae4fd
Merge branch 'fix-repeater-collapsible-on-push' of github.com:blackba…
Blackbaud-SteveBrush Jun 5, 2018
31c4fa2
addressed PR comments
blackbaud-conorwright Jun 8, 2018
fee4cf2
removed observable. Triggered change mark
blackbaud-conorwright Jun 11, 2018
97a61c2
reordered imports
blackbaud-conorwright Jun 12, 2018
48bffe8
Merge remote-tracking branch 'upstream/master' into fix-repeater-coll…
blackbaud-conorwright Jun 12, 2018
b4b1437
Merge branch 'master' into fix-repeater-collapsible-on-push
Blackbaud-SteveBrush Jun 12, 2018
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
8 changes: 6 additions & 2 deletions src/demos/repeater/repeater-demo.component.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import { Component } from '@angular/core';
import {
ChangeDetectionStrategy,
Component
} from '@angular/core';

@Component({
selector: 'sky-repeater-demo',
templateUrl: './repeater-demo.component.html'
templateUrl: './repeater-demo.component.html',
changeDetection: ChangeDetectionStrategy.OnPush
})
export class SkyRepeaterDemoComponent {
public items: any[];
Expand Down
4 changes: 3 additions & 1 deletion src/modules/repeater/repeater-item-content.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { Component } from '@angular/core';
import {
Component
} from '@angular/core';

@Component({
selector: 'sky-repeater-item-content',
Expand Down
4 changes: 3 additions & 1 deletion src/modules/repeater/repeater-item-context-menu.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { Component } from '@angular/core';
import {
Component
} from '@angular/core';

@Component({
selector: 'sky-repeater-item-context-menu',
Expand Down
4 changes: 3 additions & 1 deletion src/modules/repeater/repeater-item-title.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { Component } from '@angular/core';
import {
Component
} from '@angular/core';

@Component({
selector: 'sky-repeater-item-title',
Expand Down
4 changes: 2 additions & 2 deletions src/modules/repeater/repeater-item.component.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<section
class="sky-repeater-item"
[ngClass]="{'sky-repeater-item-collapsible': isCollapsible, 'sky-repeater-item-selected': isSelected}"
[ngClass]="{'sky-repeater-item-collapsible': (isCollapsibleObservable | async), 'sky-repeater-item-selected': isSelected}"
>

<div class="sky-repeater-item-left">
Expand Down Expand Up @@ -29,7 +29,7 @@
<h1 class="sky-repeater-item-title" #titleEl>
<ng-content select="sky-repeater-item-title"></ng-content>
</h1>
<div class="sky-repeater-item-chevron" [hidden]="!isCollapsible">
<div class="sky-repeater-item-chevron" [hidden]="!(isCollapsibleObservable | async)">
<sky-chevron
[direction]="isExpanded ? 'up' : 'down'"
(directionChange)="chevronDirectionChange($event)"
Expand Down
36 changes: 26 additions & 10 deletions src/modules/repeater/repeater-item.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,23 @@ import {
Input
} from '@angular/core';

import { skyAnimationSlide } from '../animation/slide';
import {
BehaviorSubject
} from 'rxjs/BehaviorSubject';

import { SkyRepeaterService } from './repeater.service';
import { SkyLogService } from '../log/log.service';
import { SkyCheckboxChange } from '../checkbox/checkbox.component';
import {
skyAnimationSlide
} from '../animation/slide';
import {

Choose a reason for hiding this comment

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

I know these imports were incorrectly formatted to begin with, but could we order them from external to local files? Also, each import needs a newline after it:
#1647 (comment)

import {
  skyAnimationSlide
} from '../animation/slide';

import {
  SkyCheckboxChange
} from '../checkbox/checkbox.component';

import {
  SkyLogService
} from '../log/log.service';

import {
  SkyRepeaterService
} from './repeater.service';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

SkyRepeaterService
} from './repeater.service';
import {
SkyLogService
} from '../log/log.service';
import {
SkyCheckboxChange
} from '../checkbox/checkbox.component';
import { Observable } from 'rxjs/Observable';

@Component({
selector: 'sky-repeater-item',
Expand All @@ -16,6 +28,7 @@ import { SkyCheckboxChange } from '../checkbox/checkbox.component';
animations: [skyAnimationSlide]
})
export class SkyRepeaterItemComponent {

public get isExpanded(): boolean {
return this._isExpanded;
}
Expand All @@ -34,22 +47,25 @@ export class SkyRepeaterItemComponent {

public slideDirection: string;

public get isCollapsible(): boolean {
return this._isCollapsible;
public get isCollapsibleObservable(): Observable<boolean> {

Choose a reason for hiding this comment

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

We've been trying to avoid Observables whenever we can because they add a layer of complexity that's difficult to debug later on. I was able to get this working without Observables, if I changed this component's change detection strategy to OnPush and used this.changeDetector.markForCheck() in the updateForExpanded() method.

Curious if that works on your end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that does work. That is a cleaner implementation

return this._isCollapsible.asObservable();
}

public get isCollapsible(): boolean {
return this._isCollapsible.getValue();
}
public set isCollapsible(value: boolean) {
if (this._isCollapsible !== value) {
this._isCollapsible = value;
if (this.isCollapsible !== value) {
this._isCollapsible.next(value);

/*istanbul ignore else */
if (!this._isCollapsible) {
if (!value) {
this.updateForExpanded(true, false);
}
}
}

private _isCollapsible = true;
private _isCollapsible: BehaviorSubject<boolean> = new BehaviorSubject(true);

private _isExpanded = true;

Expand Down