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

feat: lists #530

Merged
merged 146 commits into from
Aug 1, 2018
Merged

feat: lists #530

merged 146 commits into from
Aug 1, 2018

Conversation

yggg
Copy link
Contributor

@yggg yggg commented Jun 27, 2018

Please read and mark the following check list before creating a pull request:

Short description of what this resolves:

List and infinite list components

'nbscroll',
{ detail: { originalEvent: $event, ...this.getScrollInfo() } },
);
this.document.dispatchEvent(event);
Copy link
Collaborator

Choose a reason for hiding this comment

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

document or better window?

Copy link
Collaborator

Choose a reason for hiding this comment

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

should we run it outside of zone?

* `nb-list-item` accepts arbitrary content, so you can create list of any components.
*
* List of users:
* @stacked-example(Users list, list/users-list-showcase.component)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@theme block is missed

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

@@ -73,6 +73,13 @@ export class NbCardFooterComponent {
* Card with header and footer:
* @stacked-example(With Header & Footer, card/card-full.component)
*
* Most of the time main card content goes to `nb-card-body`, so it styled and aligned according to header and footer.
Copy link
Collaborator

Choose a reason for hiding this comment

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

so it is styled and aligned according_ly_ to header and footer

@@ -73,6 +73,13 @@ export class NbCardFooterComponent {
* Card with header and footer:
* @stacked-example(With Header & Footer, card/card-full.component)
*
* Most of the time main card content goes to `nb-card-body`, so it styled and aligned according to header and footer.
* But in case you need a higher level of control, you can pass contend directly to `nb-card`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

But In

yggg added 28 commits July 11, 2018 21:42
@codecov
Copy link

codecov bot commented Jul 30, 2018

Codecov Report

Merging #530 into master will increase coverage by 0.77%.
The diff coverage is 92%.

@@            Coverage Diff            @@
##           master    #530      +/-   ##
=========================================
+ Coverage   69.72%   70.5%   +0.77%     
=========================================
  Files         120     124       +4     
  Lines        3465    3590     +125     
  Branches      247     260      +13     
=========================================
+ Hits         2416    2531     +115     
- Misses       1003    1010       +7     
- Partials       46      49       +3
Impacted Files Coverage Δ
.../framework/theme/components/card/card.component.ts 68.75% <ø> (ø) ⬆️
.../framework/theme/components/list/list.component.ts 100% <100%> (ø)
src/framework/theme/components/list/list.module.ts 100% <100%> (ø)
...k/theme/components/list/infinite-list.directive.ts 90.16% <90.16%> (ø)
...eme/components/list/list-page-tracker.directive.ts 92.15% <92.15%> (ø)

@@ -348,6 +365,7 @@ export const NB_EXAMPLE_COMPONENTS = [
NbPlaygroundBaseComponent,
...NB_EXAMPLE_COMPONENTS,
],
providers: [ NewsService ],
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we move it on a component level?

@@ -73,6 +73,13 @@ export class NbCardFooterComponent {
* Card with header and footer:
* @stacked-example(With Header & Footer, card/card-full.component)
*
* Most of the time main card content goes to `nb-card-body`, so it styled and aligned accordingly to header and footer.
Copy link
Collaborator

Choose a reason for hiding this comment

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

so it is styled and aligned in accordance with the header and footer.

private getContainerDimentions(): Observable<NbScrollableContainerDimentions> {
if (this.elementScroll) {
const { scrollTop, scrollHeight, clientHeight } = this.elementRef.nativeElement;
return of({ scrollTop, scrollHeight, clientHeight });
Copy link
Collaborator

Choose a reason for hiding this comment

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

of -> observableOf

) {
if (isPlatformBrowser(this.platformId) && this.window.history.scrollRestoration) {
this.initialScrollRestoration = window.history.scrollRestoration;
history.scrollRestoration = 'manual';
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should add a not about this

}

updateUrl(page) {
const queryParams = { ...this.route.snapshot.queryParams, page };
Copy link
Collaborator

Choose a reason for hiding this comment

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

we might not need to merge it here as navigate can do this internally https://angular.io/api/router/NavigationExtras#queryParamsHandling

}

[class$="placeholder"] {
background: #f1f2f3;
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about cosmic?

nnixaa
nnixaa previously approved these changes Aug 1, 2018
@nnixaa nnixaa merged commit a07c78c into akveo:master Aug 1, 2018
@nnixaa
Copy link
Collaborator

nnixaa commented Aug 1, 2018

@yggg 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants