Skip to content

HPv3: Fixes, Pixel Perfect, Responsive#1232

Merged
ryolambert merged 28 commits intomainfrom
feat/NFT-1873-animation
Apr 20, 2023
Merged

HPv3: Fixes, Pixel Perfect, Responsive#1232
ryolambert merged 28 commits intomainfrom
feat/NFT-1873-animation

Conversation

@aglovlyuk
Copy link
Copy Markdown
Collaborator

Describe your changes

  • Pixel Perfect
  • Responsive fixes

Associated JIRA task link

  • NFT-1873

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 18, 2023

@aglovlyuk is attempting to deploy a commit to the NFT-com Team on Vercel.

To accomplish this, @aglovlyuk needs to request access to the Team.

Afterwards, an owner of the Team is required to accept their membership request.

If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account.

# Conflicts:
#	components/modules/HomePage/DiscoverCollections.tsx
#	components/modules/HomePage/HeroSection.tsx
#	components/modules/HomePage/SocialSection.tsx
#	components/modules/HomePage/WhatWeCanDo.tsx
#	pages/index.tsx
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 18, 2023

@dimamankov is attempting to deploy a commit to the NFT-com Team on Vercel.

To accomplish this, @dimamankov needs to request access to the Team.

Afterwards, an owner of the Team is required to accept their membership request.

If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account.

Copy link
Copy Markdown
Contributor

@lgtmrobo lgtmrobo left a comment

Choose a reason for hiding this comment

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

Please see comments, footer changes need to be gated until new homepage v3 goes out

footer from your PR:
Screenshot 2023-04-18 at 3 49 30 PM

Footer on Live site:
Screenshot 2023-04-18 at 3 49 21 PM


// Marquees
gsap
.timeline({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does removing this affect things on the current homepage? Should these changes be gated?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We have moved old HP to another file HomePageV2.tsx'. Changes have been gated.

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
production-interface ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 19, 2023 11:57pm
sandbox-interface ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 19, 2023 11:57pm
staging-interface ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 19, 2023 11:57pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
storybook ⬜️ Ignored (Inspect) Apr 19, 2023 11:57pm

'minlg:before:w-40 minlg:before:absolute minlg:before:left-[2.5rem] minlg:before:top-0 minlg:before:h-[1.875rem] before:bg-white before:skew-x-[-20deg]',
'minlg:after:w-[8.9rem] minlg:after:absolute minlg:after:left-[11.5rem] minlg:after:top-0 minlg:after:h-[5.5rem] after:bg-white after:skew-x-[-20deg]'
)}>
<svg className='absolute left-7 top-0 minlg:hidden' width="239" height="92" viewBox="0 0 239 92" fill="white" xmlns="http://www.w3.org/2000/svg">
Copy link
Copy Markdown
Contributor

@lgtmrobo lgtmrobo Apr 18, 2023

Choose a reason for hiding this comment

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

@ryolambert any thoughts here for optimization for this svg?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Honestly, not much room to optimize. Though preferably, this should be moved to it's own icon file public/icons/svg-name.svg, then imported as import ExSvgIcon from 'public/icons/svg-name.svg?svgr';. Unless we've got any state that's linked to directly changing the svg's styling for colors, etc.

Once the svg is in it's own separate file we can run a tool like svgo on it to minify it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done: own icon file

@aglovlyuk
Copy link
Copy Markdown
Collaborator Author

Please see comments, footer changes need to be gated until new homepage v3 goes out

footer from your PR: Screenshot 2023-04-18 at 3 49 30 PM

Footer on Live site: Screenshot 2023-04-18 at 3 49 21 PM

Done

Copy link
Copy Markdown
Contributor

@lgtmrobo lgtmrobo left a comment

Choose a reason for hiding this comment

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

@dimamankov some comments, but getting there. Also I still see a few more places where the button needs to be added ex BuildProfile.tsx. Please check all components for homepage v3

SECONDARY = 'SECONDARY'
}
export enum WebButtonSize {
DEFAULT = 'DEFAULT',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this is needed, isn't there only one web button size?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Check please new commit

switch (props.size) {
case WebButtonSize.DEFAULT:
switch(props.type) {
case(ButtonType.PRIMARY):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't this be WebButtonType.PRIMARY ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Check please new commit

Copy link
Copy Markdown
Contributor

@lgtmrobo lgtmrobo left a comment

Choose a reason for hiding this comment

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

Overall looks good, there are still a couple of things to fix in my opinion. There is still also a rogue tag being used in ProfileSection.tsx. If these are fixed, you should be good from my side! Thanks for the hard work

Copy link
Copy Markdown
Contributor

@ryolambert ryolambert left a comment

Choose a reason for hiding this comment

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

@dimamankov Looks good to me, let's merge and we can iterate on any hot fixes/design changes necessary. 👍

data-aos-delay="400"
>
<BlurImage
width={700}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@dimamankov Also recommend adding a drop shadow to the images to match the designs. 👍

Suggested change
width={700}
className='drop-shadow-xl'
width={700}

Copy link
Copy Markdown
Contributor

@lgtmrobo lgtmrobo left a comment

Choose a reason for hiding this comment

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

all clear on my end, good work

@ryolambert ryolambert merged commit 46b461a into main Apr 20, 2023
@ryolambert ryolambert deleted the feat/NFT-1873-animation branch April 20, 2023 15:48
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.

4 participants