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/nft showcase preview #371

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

superzordon
Copy link
Contributor

Shouldn't be deployed until deso-protocol/backend#98 is deployed

@superzordon superzordon requested a review from a team as a code owner August 6, 2021 21:56
@@ -64,6 +64,7 @@ export class BackendRoutes {
static RoutePathGetNFTsForUser = "/api/v0/get-nfts-for-user";
static RoutePathGetNFTBidsForUser = "/api/v0/get-nft-bids-for-user";
static RoutePathGetNFTShowcase = "/api/v0/get-nft-showcase";
static RoutePathGetNFTShowcasePreview = "/api/v0/admin/get-nft-showcase-preview";
Copy link
Member

Choose a reason for hiding this comment

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

move this to live with the rest of the admin routes below.

src/app/nft-drop-mgr/nft-drop-mgr.component.ts Outdated Show resolved Hide resolved
src/app/nft-drop-mgr/nft-drop-mgr.component.ts Outdated Show resolved Hide resolved
this.activeTab = tab;
this.router.navigate([], {
relativeTo: this.route,
queryParams: { feedTab: this.activeTab },
Copy link
Member

Choose a reason for hiding this comment

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

let's not call the query param feedTab. I think we call it feedTab in the feed component and this could cause some issues when merging queryParams.

Comment on lines 68 to 82
showShowcaseManagementTab() {
return (
this.activeTab === NftDropMgrComponent.SHOWCASE_MANAGEMENT &&
this.posts.length > 0 &&
(!this.loading || this.loadingNewDrop)
);
}

showShowcasePreviewTab() {
return (
this.activeTab === NftDropMgrComponent.SHOWCASE_PREVIEW_TAB &&
this.posts.length > 0 &&
(!this.loading || this.loadingNewDrop)
);
}
Copy link
Member

Choose a reason for hiding this comment

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

we should avoid using these functions to determine what should be shown. Instead, we should have a container div that is conditionally shown *ngIf="posts.length && (!loading || loadingNewDrop)", inside of which we show the showcase management *ngIf="activeTab === NftDropMgrComponent.SHOWCASE_MANAGEMENT" and show the preview *ngIf="activeTab === NftDropMgrComponent.SHOWCASE_PREVIEW_TAB"

@@ -1,14 +1,17 @@
import { Component, OnInit } from "@angular/core";
import {Component, Input, OnInit} from "@angular/core";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import {Component, Input, OnInit} from "@angular/core";
import { Component, Input, OnInit } from "@angular/core";

Comment on lines 38 to 41
showcaseComingSoon() {
return !this.loading && (!this.nftCollections || !has(this.nftCollections, length));
}

Copy link
Member

Choose a reason for hiding this comment

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

we should be able to kill this function and just use the constants to define the conditional

import { BackendApiService, NFTCollectionResponse } from "../backend-api.service";
import { GlobalVarsService } from "../global-vars.service";
import { InfiniteScroller } from "../infinite-scroller";
import { IAdapter, IDatasource } from "ngx-ui-scroll";
import { has } from "lodash";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import { has } from "lodash";

Comment on lines 92 to 94
console.log('Here is the showcase idx');
console.log(this.showcaseIdx);
console.log(this.showcaseIdx === undefined);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
console.log('Here is the showcase idx');
console.log(this.showcaseIdx);
console.log(this.showcaseIdx === undefined);

<div *ngIf="posts.length > 0 && (!loading || loadingNewDrop)" class="p-15px fs-15px">
<tab-selector [tabs]="feedTabs" [activeTab]="activeTab" (tabClick)="_handleTabClick($event)"></tab-selector>
<!--Admin panel-->
<div *ngIf="showShowcaseManagementTab()" class="p-15px fs-15px">
<b *ngIf="!loadingNewDrop">NFTs in this drop:</b>
<div #uiScroll *uiScroll="let post of datasource" [ngClass]="{'d-flex align-items-center mt-15px border border-color-grey br-8px': !loadingNewDrop}">
Copy link
Member

Choose a reason for hiding this comment

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

this uiScroll directive needs to be moved into a separate component. If you switch back and forth between the tabs, you're going to lose the Showcase Management view. You'll see an error that looks like the below:

ERROR TypeError: Cannot read property 'dispose' of undefined
    at UiScrollComponent.ngOnDestroy (ngx-ui-scroll.js:57)
    at executeOnDestroys (core.js:7405)
    at cleanUpView (core.js:7318)
    at destroyViewTree (core.js:7144)
    at destroyLView (core.js:7296)
    at ViewContainerRef.remove (core.js:23208)
    at ViewContainerRef.clear (core.js:23116)
    at NgIf._updateView (common.js:3535)
    at NgIf.set ngIf [as ngIf] (common.js:3502)
    at setInputsForProperty (core.js:10961)

@superzordon superzordon requested a review from lazynina August 10, 2021 19:30
@lazynina
Copy link
Member

some weird behavior when switching between drops still. let's revisit this later.

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