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.

Sign up for GitHub

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

Jump to bottom

#4276 Landing page visual change #4440

Merged
merged 11 commits into from
Dec 8, 2022
Merged

#4276 Landing page visual change #4440

merged 11 commits into from
Dec 8, 2022

Conversation

prachi00
Copy link
Member

@prachi00 prachi00 commented Dec 3, 2022 •
edited

Thank you for your contribution to the KodaDot NFT gallery.

👇 _ Let's make a quick check before the contribution.

PR Type

  • Bugfix
  • Feature
  • Refactoring

Context

Before submitting pull request, please make sure:

  • My contribution builds clean without any errors or warnings
  • I've merged recent default branch -- main and I've no conflicts
  • I've tried to respect high code quality standards
  • I've didn't break any original functionality
  • I've posted a screenshot of demonstrated change in this PR

Optional

  • I've tested it at </bsx/collection>
  • I've tested PR on mobile
  • I've written unit tests 🧪
  • I've found edge cases

Had issue bounty label?

  • Fill up your KSM address:
    Payout

Community participation

Screenshot 📸

  • My fix has changed something on UI; a screenshot is best to understand changes for others.
    Screen Shot 2022-12-03 at 3 39 39 PM
    Screen Shot 2022-12-03 at 3 39 46 PM

@prachi00 prachi00 requested a review from a team as a code owner December 3, 2022 23:40
@prachi00 prachi00 requested review from preschian and removed request for a team December 3, 2022 23:40
@kodabot
Copy link
Collaborator

kodabot commented Dec 3, 2022

WARNING @prachi00 PR for issue #4276 which isn't assigned to you. Please be warned that this PR may get rejected if there's another assignee for issue #4276

@netlify
Copy link

netlify bot commented Dec 3, 2022 •
edited

✅ Deploy Preview for koda-nuxt ready!

Name Link
🔨 Latest commit ca51d44
🔍 Latest deploy log https://app.netlify.com/sites/koda-nuxt/deploys/6391e1cdc7c0260008d0bded
😎 Deploy Preview https://deploy-preview-4440--koda-nuxt.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@yangwao
Copy link
Member

yangwao commented Dec 4, 2022 •
edited

ooo looks really nice! :)

will wait from @exezbcz to visual check

@exezbcz
Copy link
Member

exezbcz commented Dec 4, 2022

It's near, but it's not it, especially on smaller displays. Perhaps it would be good to separate the components, the blurry shapes, and then the graphics. As a result, it can be scaled and turned off separately. - I'll export it and name it accordingly. The visuals would then behave just as they do on the present page (not showing on smaller screens)
image

  • here, i would only show the blurred shapes.
  • they are also cropped badly, my fault - in latest export it should be correct.
    image

image
it would be also nice staying from this behavior - graphics collapsing into the heading.
it can maybe scale down?
image

new svgs - again, sorry for that, lmk if it is working properly
light mode left
light mode right
light mode shapes left
light mode shapes right
dark mode left
dark mode right
dark mode shapes left
dark mode shapes right

Thanks!

@exezbcz
Copy link
Member

exezbcz commented Dec 4, 2022

one more thing, please dont constraint the blurred shapes only to the footer
image

@yangwao yangwao added the S-changes-requested-🤞 PR is almost good to go, just some fine tunning label Dec 4, 2022
@prachi00
Copy link
Member Author

prachi00 commented Dec 5, 2022

@exezbcz for the new svgs, why are there four blurred shapes? and also the shapes for light mode is missing

@exezbcz
Copy link
Member

exezbcz commented Dec 5, 2022

@exezbcz for the new svgs, why are there four blurred shapes? and also the shapes for light mode is missing

yes, i was exporting that in bulk, its excessive.
Shapes for light mode are also in the comment.
image

@prachi00
Copy link
Member Author

prachi00 commented Dec 6, 2022

Screen Shot 2022-12-05 at 9 49 22 PM

I am still trying to figure out the footer thing, because if I remove the z-index, while it shows correctly, it makes see more and the 1st article non clickable, so trying to find a way around this but rest of the changes I pushed

@exezbcz
Copy link
Member

exezbcz commented Dec 6, 2022

footer graphics missing on darkmode

@prachi00
Copy link
Member Author

prachi00 commented Dec 7, 2022

footer graphics missing on darkmode

fixed and the bg issue is fixed too

preschian
preschian requested changes Dec 7, 2022
View reviewed changes
Copy link
Member

@preschian preschian left a comment

Choose a reason for hiding this comment

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

fix deepscan issue please

@preschian
Copy link
Member

@exezbcz on hover barely to see the text, is that expected?
image

@prachi00 this will affect all pages. this section will not clickable. try to figure out find another approach
image

@exezbcz
Copy link
Member

exezbcz commented Dec 7, 2022 •
edited

@exezbcz on hover barely to see the text, is that expected?

haha, It took me a while to notice it :DDD
Yup, thats a problem. First issue I see is that there should not be hover offect on text like this. I never mentioned it anywhere. I should have raised an issue for that, my bad.

@exezbcz exezbcz mentioned this pull request Dec 7, 2022
Closed
@prachi00
Copy link
Member Author

prachi00 commented Dec 8, 2022

@preschian I have removed that code for now
@exezbcz can we keep the footer as it is right now and create a new issue to make it overflow to the top, I am facing issues with it, so not really sure how to go on about doing that

@yangwao yangwao mentioned this pull request Dec 8, 2022
Closed
@yangwao
Copy link
Member

yangwao commented Dec 8, 2022

can we keep the footer as it is right now and create a new issue

yes, otherwise lgtm for me

@codeclimate
Copy link

codeclimate bot commented Dec 8, 2022

Code Climate has analyzed commit ca51d44 and detected 0 issues on this pull request.

View more on Code Climate.

@yangwao
Copy link
Member

yangwao commented Dec 8, 2022

pay 80 usd

@yangwao yangwao merged commit e206954 into main Dec 8, 2022
@yangwao yangwao deleted the feat-ladning-new branch December 8, 2022 13:24
@yangwao
Copy link
Member

yangwao commented Dec 8, 2022

😍 Perfect, I’ve sent the payout
💵 $80 @ 29.37 USD/KSM ~ 2.724 $KSM
🧗 EzGc4s9PgCPx1YnF3fqzhLzVHpHMTL4LWPScwpDrR8JKgSU
🔗 0x8dbc19005137754947074f573ca80034735199b9504142089655dc37c174e687

🪅 Let’s grab another issue and get rewarded!
🪄 github.com/kodadot/nft-gallery/issues

@yangwao yangwao added the paid pull-request has been paid label Dec 8, 2022
@preschian
Copy link
Member

sorry, I missed the test, could you check if the test failed related to this PR or not? https://github.com/kodadot/nft-gallery/actions/runs/3648561364/jobs/6162326283
cc @prachi00

This was referenced Dec 9, 2022
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@preschian preschian preschian approved these changes

Assignees
No one assigned
Labels
paid pull-request has been paid S-changes-requested-🤞 PR is almost good to go, just some fine tunning
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Landing page visual change
5 participants
@prachi00 @kodabot @yangwao @exezbcz @preschian