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

[TAS-158] ✨ Revamp index page #1288

Merged
merged 28 commits into from
Aug 22, 2023
Merged

[TAS-158] ✨ Revamp index page #1288

merged 28 commits into from
Aug 22, 2023

Conversation

nwingt
Copy link
Member

@nwingt nwingt commented Aug 8, 2023

No description provided.

@notion-workspace
Copy link

Implement new index page

nwingt added a commit that referenced this pull request Aug 8, 2023
nwingt added a commit that referenced this pull request Aug 8, 2023
nwingt added a commit that referenced this pull request Aug 10, 2023
nwingt added a commit that referenced this pull request Aug 10, 2023
nwingt added a commit that referenced this pull request Aug 11, 2023
nwingt added a commit that referenced this pull request Aug 16, 2023
nwingt added a commit that referenced this pull request Aug 17, 2023
nwingt added a commit that referenced this pull request Aug 17, 2023
@nwingt nwingt marked this pull request as ready for review August 17, 2023 14:41
nwingt added a commit that referenced this pull request Aug 17, 2023
nwingt added a commit that referenced this pull request Aug 18, 2023
nwingt added a commit that referenced this pull request Aug 18, 2023
nwingt added a commit that referenced this pull request Aug 18, 2023
nwingt added a commit that referenced this pull request Aug 18, 2023
src/pages/nft/class/_classId/index.vue Outdated Show resolved Hide resolved
src/pages/nft/class/_classId/_nftId.vue Outdated Show resolved Hide resolved
src/layouts/default.vue Outdated Show resolved Hide resolved
src/pages/index.vue Outdated Show resolved Hide resolved
nwingt added a commit that referenced this pull request Aug 19, 2023
@@ -1457,10 +1457,15 @@ export default {
},
},
mounted() {
window.addEventListener('resize', this.handleResize);
this.resizeListener = window.addEventListener('resize', this.handleResize);
Copy link
Member

Choose a reason for hiding this comment

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

Should define resizeListener in data?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think no need to put it in data for reactivity

Copy link
Member

Choose a reason for hiding this comment

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

Best to define it somewhere clearly as a class atteibute though

Copy link
Member Author

Choose a reason for hiding this comment

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

Defined in $options

this.handleResize();
this.animate();
},
beforeDestroy() {
if (this.resizeListener) {
window.removeEventListener(this.resizeListener);
Copy link
Member

Choose a reason for hiding this comment

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

Best to set this.resizeListener as null after removing it

@nwingt nwingt merged commit 6db4177 into develop Aug 22, 2023
1 of 2 checks passed
@nwingt nwingt deleted the feature/index-revamp branch August 22, 2023 08:02
williamchong added a commit that referenced this pull request Aug 28, 2023
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.

None yet

2 participants