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

Featured nft section, marketplace view #4087

Merged
merged 13 commits into from
Apr 28, 2023

Conversation

drillprop
Copy link
Contributor

Fix #3776

to test it, go to FeaturedNftSection and adjust the number of items, for example:

  // const items = loading ? createPlaceholderData(10) : nfts ?? []
  const items = createPlaceholderData(15)

Copy link
Contributor

@WRadoslaw WRadoslaw left a comment

Choose a reason for hiding this comment

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

LGTM

@drillprop drillprop marked this pull request as draft April 21, 2023 10:01
@drillprop
Copy link
Contributor Author

Actually, I think I broke the section with the carousel with all these changes. Draft

@drillprop drillprop added the don't merge PRs that couldn't be merged for some reason label Apr 21, 2023
@drillprop
Copy link
Contributor Author

drillprop commented Apr 24, 2023

To test this, go to https://atlas-jf3dq6nt5-joystream.vercel.app/marketplace

My goal was to make the carousel overflow its container. Like on the screens below:
image
To accomplish this I adjusted a few styles and set paddings and negative margins based on the position of the carousel. The main problem is we're using an external library called Swiper and if we change something in terms of container sizing the library is miscalculating things(I suppose)

The problems with the current implementation:

  • If we want to make the carousel overflow its container it's impossible to adjust the number of columns - the current approach is always to use grid "auto". To make this possible we need to specify the fixed width of every item and I suppose we need to specify this for every breakpoint. Currently, it's 300 px for all breakpoints.
  • button in the header is not switching to disabled once you reach the end. Don't know why
    image
  • when you click the next slide button, once you reach the end, the last click on the button moves the carousel just a tiny amount.
  • the last item in the carousel is not well aligned with the edge of the container:
    image
  • every prop change for the swiper requires a refreshing site. You cannot change the prop dynamically

@drillprop drillprop changed the title Featured nft section Featured nft section, marketplace view Apr 25, 2023
@drillprop drillprop marked this pull request as ready for review April 25, 2023 07:44
@drillprop
Copy link
Contributor Author

drillprop commented Apr 25, 2023

In the end, I didn't manage to accomplish the design requirements in terms of the carousel. The carousel won't be overflowing its container.
@attemka or @WRadoslaw perhaps It's worth rereview this PR once again because I added some changes in terms of marketplace layout

@attemka attemka merged commit bd03b22 into Joystream:dev Apr 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.

3 participants