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

feat: revamp stacks nfts send to screen #653

Conversation

teebszet
Copy link
Member

@teebszet teebszet commented Nov 10, 2023

πŸ”˜ PR Type

  • Bugfix
  • Enhancement

πŸ“œ Background

https://linear.app/xverseapp/issue/ENG-3188/send-stx-nft-screen-in-fullscreen

πŸ”„ Changes

  • fix: ledger accounts should only open in new tab when not already in full screen
  • fix: back button display should rely on back history not ledger account
  • feat: revamp the send nfts screen to match send ordinals layout (responsive)

Impact:

  • send nft screen and send ordinal were refactored to share the same layout, so can check for layout breakages and functional breakages on both screens (extension and full screen view)
  • any flow which can open a new tab while using ledger account, now only opens the new tab if not already in full screen
    • any Send button
    • any Add Stacks Address button
    • recover assets button from settings page
  • both send nft and send ordinal screens were previously hiding back button if ledger account. but if the user did 'open gallery' while on the collectibles tabs page, and then clicked through to 'send', we should allow the user to go back

πŸ–Ό Screenshot / πŸ“Ή Video

Screen.Recording.2023-11-10.at.6.48.43.PM.mov

βœ… Review checklist

Please ensure the following are true before merging:

  • Code Style is consistent with the project guidelines.
  • Code is readable and well-commented.
  • No unnecessary or debugging code has been added.
  • Security considerations have been taken into account.
  • The change has been manually tested and works as expected.
  • Breaking changes and their impacts have been considered and documented.
  • Code does not introduce new technical debt or issues.

@teebszet teebszet marked this pull request as draft November 10, 2023 07:56
@teebszet teebszet changed the title Tim/eng 2815 stacks nfts send to feat: revamp stacks nfts send to screen Nov 10, 2023
@teebszet teebszet marked this pull request as ready for review November 10, 2023 10:44
and debounce the stx address -> bns name resolver
)}
{children}
</Container>
{isScreenLargerThanXs && (
Copy link
Contributor

@fedeerbes fedeerbes Nov 10, 2023

Choose a reason for hiding this comment

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

it will be cool to have a component which handles rendering according to break points, cause i see a lot of this conditional renders with the variable defined above. something like

<RenderBPLayout
    xs={<Component/>}
    m={ <Component/>}
    />

or

<RenderBPLayout
    >
      {(breakpoint) => {
        switch (breakpoint) {
          case 'xs':
            return <Component/>;
          case 'm':
            return <Component/>;
          default:
            return <Component/>;
      }
      }
    </RenderBPLayout>

}));

const Button = styled.button((props) => ({
const AmountInputContainer = styled.div<{ error: boolean }>((props) => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

we should consider making components from this to avoid code repetition, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah true. could be the right time to make the input component in Ui library

@teebszet teebszet merged commit 2e49ba3 into imamahzafar/eng-2807-stacks-nft-tab-data-fetching-pagination-and-load-more-button Nov 13, 2023
@teebszet teebszet deleted the tim/eng-2815-stacks-nfts-send-to branch November 13, 2023 08:31
teebszet added a commit that referenced this pull request Nov 21, 2023
* Add hook to get nft collection data

* Update stx collectible UI

* Make NFT collage for collections

* Fix on click function

* fix import

* Update NFT Collection Page & NFT Detail Page (#633)

* Update NFT Detail UI

* Use hook to get collection data

* Add NFT Collection screen

* Fix navigation

* Remove hard coded address

* Fix bns name navigation

* refactor: use array for cache key

* Turn off no-nested-ternary rule globally

* Move nft util functions to a seperate file

* fix: remove unnecessary use mutation in nft details

and some other bug fixes

* fix: add the useResetUserFlow hook to nft collection page

* refactor: remove nftData from redux store

use the react-query cache instead, and fixes bug where some nftData was
not being stored

* fix: add isLoading state back to nftdetail and route to send with id

* fix: react console errors and send nft finishes at dashboard screen

* feat: add sticky tabs to collectibles dashboard

* refactor: separate use nft dashboard to separate file

* fix: bns detail page works

* chore: use core version with some updates

* fix: make bns names in collection screen not clickable and display name

* chore: enable stx test address from localstorage

* fix: button width for gallery view (#650)

* fix: open NFT gamma link (#649)

* fix: open nft page on gamma

* refactor: gamma url

* fix: metaData

* fix: show snackbar if NFT metadata is failed to fetch (#648)

* fix: show snackbar if nft metadata is failed to fetch

* fix: snackbar styling

* feat: revamp stacks nfts send to screen (#653)

* feat: revamp the send nft screen

* fix: back button display should rely on back history not ledger account

* fix: ledger accounts should only open in new tab when not already in full screen

otherwise it is annoying

* fix: add the bns name resolver to send nft form

* fix: opening a send-nft from ledger should open responsive screen

and debounce the stx address -> bns name resolver

* fix: some nft images are still showing the moon loader spinner it should be (#654)

* chore: tweak BetterBarLoader component to receive string as width and height in order to user percentages

* fix: key prop in nft collection screen

* fix: hardcoded placeholder size values and replace error icon with the new one

* fix: NFT detail screen UI (#655)

* fix: detail screen UI

* fix: UI alignments

* fix: buttons spacing

* feat: hook items count with correct values (#658)

* fix: column gap on tiles skeleton loader

* chore: update core version

* chore: follow i18n next doc about plurals (#661)

* fix: extension crashes when closing the broadcast success screen (#662)

* fix: wrong background color on transaction status screens

* fix: wrong navigation routes on transaction status screen

* fix: copy on inscription complete screen

* fix: z index on sticky tabs list and modals, popups (#665)

* refactor: collectibles fetch (#666)

* refactor: adapt to core refactor of stx collectibles

* fix: add back the check for metadata before nav to nft detail screen

* fix: use 24 hour staleTime for nft details to match mobile

* refactor: remove unused utils function

* fix: add react-is-visible to virtualise nft tab and nft collection page

* chore: bump core version with duplicate nft fix

* chore: bump core version

* chore: update core version and remove total_nft

* fix: bns image size and disable click on bns item (#668)

---------

Co-authored-by: Tim Man <tim@secretkeylabs.com>
Co-authored-by: Abdul Haseeb <haseeb4239@gmail.com>
Co-authored-by: fede erbes <fedeerbes@gmail.com>
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