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: collectibles widget #481

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Conversation

pete-watters
Copy link
Contributor

@pete-watters pete-watters commented Sep 26, 2024

This PR:

  • Adds collectibles widget to the native app using mock data
  • Migrates some similar UI components from the extension to the monorepo

Once the UI package is released I can complete leather-io/extension#5876

Mobile demo

collectibles.mp4

Mobile storybook demo

collectible-storybook.mp4

There are some remaining items to figure out:

  • An alternative for dompurify.sanitize for text ordinals
  • Stopping scroll on WebView ordinals
  • Fixing padding and margin on all widgets @edgarkhanzadian - I'll do this in

Copy link

codecov bot commented Sep 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 22.38%. Comparing base (498a1db) to head (432e828).

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #481   +/-   ##
=======================================
  Coverage   22.38%   22.38%           
=======================================
  Files         132      132           
  Lines        5521     5521           
  Branches      241      241           
=======================================
  Hits         1236     1236           
  Misses       4285     4285           
Components Coverage Δ
bitcoin 53.77% <ø> (ø)
query 12.05% <ø> (ø)
utils 52.45% <ø> (ø)
crypto 69.40% <ø> (ø)
stacks 53.27% <ø> (ø)

@pete-watters pete-watters changed the title Feat/222/collectibles widget feat: collectibles widget Sep 26, 2024
@pete-watters pete-watters force-pushed the feat/222/collectibles-widget branch 2 times, most recently from afdc14d to e8d90a8 Compare September 26, 2024 10:43
@pete-watters pete-watters reopened this Sep 27, 2024
@@ -0,0 +1,67 @@
import { Square, SquareProps } from 'leather-styles/jsx';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved this as it was the only other thing using use-events so I can also move that package here.

We use that in CollectibleItemLayout which I migrated here but could revert to the extension if desired.

The hook usePressable is only used once in the extension accounts.tsx = AddAccountAction

src: string;
}

const mockOrdinals: Ordinal[] = [
Copy link
Contributor Author

@pete-watters pete-watters Sep 27, 2024

Choose a reason for hiding this comment

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

I obtained these mocks by logging all the inscriptions we have on Leather Dev wallet and picking out different samples.

The structure here is what we are sending to the components but it seems different than the BIS docs and also what we have in leather.io/query for mock-inscriptions Today I found this other utils that is mapping BIS to this format - createBestInSlotInscription

I think it's OK for now and we can improve this and share the mocks better if needed / ditch these altogether when connecting the API.

};

export default meta;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added some mocks of what we use to actually render inscriptions here.

  • Video seems to be handled the same as html / svg in an iframe anyway.
  • Audio seems to just show an icon now

I can inscribe audio + video on ordinals bot if we think that's useful. For video the network fee is high so it's ~ $392 for what I tested. Audio can be cheaper ~ $122.02.

@pete-watters pete-watters force-pushed the feat/222/collectibles-widget branch 2 times, most recently from 64beb49 to 5951b4d Compare September 27, 2024 08:46
export function CollectibleHtml({ src }: CollectibleHtmlProps) {
return (
<CollectibleCardLayout>
<WebView source={{ uri: src }} width={200} height={200} style={{ overflow: 'hidden' }} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to do a bit more work here as I still see some scroll bars

html_scroll.mp4

@@ -0,0 +1,21 @@
import { type Collectible, type CollectibleCardProps } from '@leather.io/ui/native';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this as a way to just get only the data we need to display from the mocks.

Later I found createBestInSlotInscription which is actually giving us this data and more.

I will investigate more when hooking up APIs but maybe it could be good to have a BE proxy for this type of stuff - data we need for App Home for example.

- I tried using jsdom as a polyfill but then hit an error Can't resolve 'vm' in jsdom
- maybe we should write our own sanitizer?
*/}
{/* {content ? sanitize(content) : ''} */}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried using dompurify.sanitize here but it seems not to work on native.

I tried a polyfill of jsdom but then that failed and needed another polyfill so I need to investigate a better way.

Maybe we could run sanitize in the query instead? So that we just provide the data ready to render


if (isOrdinal) {
switch (mimeType) {
// TODO: add audio support
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't implement mimeType === 'audio' yet. It seems like on web we just show a Head phone icon unless I am missing something

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, that is correct. We didn't want the user to interact with the controls in the iframe so we just render an icon with a link to the audio.

@pete-watters pete-watters marked this pull request as ready for review September 27, 2024 09:03

import { Widget } from '../widget';

interface CollectiblesWidgetProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
interface CollectiblesWidgetProps {
interface CollectiblesWidgetLayoutProps {

Copy link
Contributor

@fbwoolf fbwoolf left a comment

Choose a reason for hiding this comment

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

Looks great, @pete-watters.

Comment on lines +3 to +19
export interface Ordinal {
id: string;
number: number;
output: string;
txid: string;
offset: string;
address: string;
preview: string;
title: string;
genesisBlockHeight: number;
genesisBlockHash: string;
genesisTimestamp: number;
value: string;
mimeType: string;
name: string;
src: string;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Think this should live in models pkg

@@ -1,6 +1,6 @@
import { motion, stagger, useAnimate } from 'framer-motion';
import { css } from 'leather-styles/css';
import { HasChildren } from 'src/utils/has-children';
import { HasChildren } from 'src/utils/has-children.shared';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you rebase on dev and make sure there are no src/ imports? Think they should cause type errors on rebase

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