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

Created the NFT component for single NFT allowance #15825

Merged
merged 3 commits into from
Oct 6, 2022

Conversation

adnansahovic
Copy link
Contributor

@adnansahovic adnansahovic commented Sep 14, 2022

Explanation

Created the NFT component for single NFT allowance. This component can't yet be added to the app. Instead, I added a storybook that displays this component.

  1. Icon displayed is the NFT collection icon
  2. Copy in bold is the NFT collection name
  3. Secondary copy is specific token ID for the NFT that is being request allowance for
  4. Clicking on "View" opens up the "View NFTs" modal: Create "view NFTs" modal #15751

More Information

Screenshots/Screencaps

Screenshot 2022-09-14 at 10 02 04

Manual Testing Steps

This component can be tested by running storybook (yarn storybook) and then go to the COMPONENTS tab and open in UI/NftInfo.

@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@adnansahovic adnansahovic self-assigned this Sep 14, 2022
@metamaskbot
Copy link
Collaborator

Builds ready [fd801cb]
Page Load Metrics (1180 ± 32 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint842681043919
domContentLoaded1068135811645828
load1068135811806732
domInteractive1068135811645828

highlights:

storybook

@mirjanaKukic
Copy link
Contributor

Verified by QA

@adnansahovic adnansahovic marked this pull request as ready for review September 14, 2022 09:47
@adnansahovic adnansahovic requested a review from a team as a code owner September 14, 2022 09:47
@adnansahovic adnansahovic requested a review from danjm September 14, 2022 09:47
textAlign={TEXT_ALIGN.END}
marginTop={4}
marginRight={4}
className="nft-info__content__view"
Copy link
Contributor

Choose a reason for hiding this comment

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

BEM class syntax doesn't allow for nesting blocks this way, if you need a block within another block then it should be named like nft-info__content-view

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @brad-decker , You can see changes here

ui/components/ui/nft-info/index.scss Outdated Show resolved Hide resolved
ui/components/ui/nft-info/nft-info.js Outdated Show resolved Hide resolved
@adnansahovic adnansahovic force-pushed the NFT-component-for-single-NFT-allowance branch from fd801cb to a8832a5 Compare September 15, 2022 11:25
@metamaskbot
Copy link
Collaborator

Builds ready [a8832a5]
Page Load Metrics (1221 ± 16 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint85150106178
domContentLoaded1133129212094120
load1133129212213416
domInteractive1133129212094120

highlights:

storybook

brad-decker
brad-decker previously approved these changes Sep 15, 2022
@metamaskbot
Copy link
Collaborator

Builds ready [b19e5dd]
Page Load Metrics (2376 ± 88 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint962261212914
domContentLoaded20272789234019192
load21002821237618488
domInteractive20272789234019192

highlights:

storybook

Copy link
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@adonesky1 adonesky1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@adonesky1 adonesky1 merged commit 6918bff into develop Oct 6, 2022
@adonesky1 adonesky1 deleted the NFT-component-for-single-NFT-allowance branch October 6, 2022 14:48
@github-actions github-actions bot locked and limited conversation to collaborators Oct 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create the NFT component for single NFT allowance
5 participants