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

FAT-131 - Stax Custom lock screen tool #743

Merged
merged 1 commit into from
Sep 6, 2022

Conversation

ofreyssinet-ledger
Copy link
Contributor

@ofreyssinet-ledger ofreyssinet-ledger commented Jul 26, 2022

📝 Description

Tool to import an image (either from the phone's gallery or from an URL) and display a cropping UI, then resize it to a target size, pick & apply a contrast value, preview the result and output the raw data.

New feature flag: "customImage"

Two entry points:

  • in the NFT viewer, by pressing the "..." button, there is a "Custom Image" button (enabled by featureflag
  • in debug settings

This is the first part of work for a bigger feature. Ask me on slack for more details about the what/why.

❓ Context

  • Impacted projects: ledger-live-mobile ledger-live-common
  • Linked resource(s): FAT-135 FAT-131

✅ Checklist

  • Test coverage
  • Atomic delivery
  • No breaking changes

📸 Demo

I can send you a video on Slack

🚀 Expectations to reach

Please make sure you follow these Important Steps.

Pull Requests must pass the CI and be internally validated in order to be merged.

@ofreyssinet-ledger ofreyssinet-ledger self-assigned this Jul 26, 2022
@vercel
Copy link

vercel bot commented Jul 26, 2022

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

Name Status Preview Updated
live-common-tools ✅ Ready (Inspect) Visit Preview Sep 6, 2022 at 9:02AM (UTC)
3 Ignored Deployments
Name Status Preview Updated
ledger-live-github-bot ⬜️ Ignored (Inspect) Sep 6, 2022 at 9:02AM (UTC)
native-ui-storybook ⬜️ Ignored (Inspect) Sep 6, 2022 at 9:02AM (UTC)
react-ui-storybook ⬜️ Ignored (Inspect) Sep 6, 2022 at 9:02AM (UTC)

@changeset-bot
Copy link

changeset-bot bot commented Jul 26, 2022

🦋 Changeset detected

Latest commit: 4de7efb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
live-mobile Patch
@ledgerhq/live-common Patch
@ledgerhq/live-cli Patch
ledger-live-desktop Patch
live-common-tools Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added mobile Has changes in LLM translations Translation files have been touched labels Jul 26, 2022
@codecov
Copy link

codecov bot commented Jul 26, 2022

Codecov Report

Merging #743 (7bf93ba) into develop (6edf507) will increase coverage by 47.84%.
The diff coverage is n/a.

❗ Current head 7bf93ba differs from pull request most recent head 4de7efb. Consider uploading reports for the commit 4de7efb to get more accurate results

@@             Coverage Diff              @@
##           develop     #743       +/-   ##
============================================
+ Coverage         0   47.84%   +47.84%     
============================================
  Files            0      650      +650     
  Lines            0    29121    +29121     
  Branches         0     7532     +7532     
============================================
+ Hits             0    13932    +13932     
- Misses           0    15131    +15131     
- Partials         0       58       +58     
Flag Coverage Δ
test 47.84% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...live-common/src/families/ripple/platformAdapter.ts 100.00% <0.00%> (ø)
libs/ledger-live-common/src/apps/formatting.ts 100.00% <0.00%> (ø)
...r-live-common/src/families/cosmos/hw-getAddress.ts 50.00% <0.00%> (ø)
...mon/src/families/solana/api/chain/account/token.ts 100.00% <0.00%> (ø)
...dger-live-common/src/exchange/swap/getKYCStatus.ts 46.66% <0.00%> (ø)
...-live-common/src/families/algorand/js-broadcast.ts 42.85% <0.00%> (ø)
libs/ledger-live-common/src/promise.ts 65.95% <0.00%> (ø)
...ledger-live-common/src/families/celo/api/hubble.ts 25.58% <0.00%> (ø)
libs/ledger-live-common/src/user.ts 100.00% <0.00%> (ø)
libs/ledger-live-common/src/hw/getVersion.ts 98.63% <0.00%> (ø)
... and 640 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@github-actions
Copy link

github-actions bot commented Jul 26, 2022

@ofreyssinet-ledger

Screenshots: ✅

There are no changes in the screenshots for this PR. If this is expected, you are good to go.

@github-actions github-actions bot added the common Has changes in live-common label Jul 26, 2022
@ofreyssinet-ledger ofreyssinet-ledger changed the title Feat/fat 131 custom image FAT-131 Custom image Jul 27, 2022
@ofreyssinet-ledger ofreyssinet-ledger changed the title FAT-131 Custom image FAT-131 Jul 27, 2022
@ofreyssinet-ledger ofreyssinet-ledger changed the title FAT-131 FAT-131 - Custom image tool Jul 27, 2022
@@ -6,7 +6,7 @@ export type FeatureId =
| "ratings"
| "counterValue"
| "buyDeviceFromLive"
| string;
Copy link
Contributor Author

@ofreyssinet-ledger ofreyssinet-ledger Jul 27, 2022

Choose a reason for hiding this comment

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

Leaving string was not strict enough in my opinion, because of that for instance we didn't detect that a default config for llmUsbFirmwareUpdate was missing.

It was introduced in this PR: LedgerHQ/ledger-live-common#1843

It also puts string as a possible type for feature flags in order to allow us to develop even without upgrading live-common.

Now that we have the mono-repo this is useless.

@@ -64,4 +64,10 @@ export const defaultFeatures: DefaultFeatures = {
counterValue: {
enabled: false,
},
llmUsbFirmwareUpdate: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was missing because the typing was not strict enough.
see https://github.com/LedgerHQ/ledger-live/pull/743/files#r930990175

@ofreyssinet-ledger ofreyssinet-ledger force-pushed the feat/FAT-131-custom-image branch 2 times, most recently from ba4a53f to 743264d Compare August 30, 2022 13:31
First poc of image processing using webview on NFTs

Migrate Webview image processor to its own component

Improve code injection using toString() & "show source" directive

Move image fetching logic to own file

Fix code injection (not calling Function.toString at compile) + add debug helper

Add button to navigate to ImagePicker in NftViewer

Add tip for code injection issue (first call of toString()) in debug

Revert changes on NftImageViewer

Add button to request compact raw data from webview script

Comment

Add props to set brightness & contrast (filter not working yet)

Working flows on iOS: (image url / pick from gallery) -> crop at given ratio -> process

Cleanup a bit the comments mess

Fix android build issue by upgrading kotlin

Add descriptive labelling in the main screen

Gallery flow working on Android

Make injected code type script

Add multiple constrast filters using manual JS filter

Avoid recomputing result twice in computeResult

lint

lint

Fix import from URL on Android by downloading image in cache

Improve display of the preview for each step (dimensions)

Image resizing to target size (after crop, before processing)

Fix expo-image-manipulator version to match expo-barcode-scanner's dependency on expo-image-loader version

Fix ImageResizer: use fileURI instead of base64 (the former doesn't work on iOS)

Fix rerenders

Store floored value in rawResult to avoid division imprecisions

Fix issues post rebase

Cleanup types of image tools

Implement BottomModal & CroppingScreen w/ navigation (WIP)

Implement PreviewScreen

Move DebugScreen to CustomImageNavigator

Implement Step3Transfer with for now just a preview of the raw data

Proper design of bottom modal

Avoid loading source image data as base64 in JS to avoid memory issues

Put back expo-file-system as it's required by expo-image-manipulator

Less extreme contrast values

post rebase on develop with RN upgrade

Add rotate button in Step1Crop

Fix contrast causing out of bound values (out of [0, 255])

Fix nav to CustomImage in NftViewer

Fix gray level rounding

Handle errors in loading flow

Add error handling & loading state in import from gallery flow

Add error handling in image preview, cropper, resizer, processor

Add error handling in injected code

Use regular image for debug link

Add data validation: rebuild image from raw data & compare base64 with preview

Fix lint & ts

Add extreme aspect ratio image

Add some alerts to debug importImageFromPhoneGallery

more debug logs

Add expo-image-picker because react-native-image-picker is buggy on samsung/xiaomi

Remove debugging stuff in image importing flow

Remove react-native-image-picker

Move CustomImage entrypoints to debug screen

Adapt cropper layout for "extreme" aspect ratio (above 2:1)

Remove ImagePicker debug screen

Remove unused variables

Add "customImage" feature flag

Update injected code doc

Remove unused component

Document components

Remove react-native-image-picker lib

Changesets

Improve typing of Step1Crop params & loadImageToFileWithDimensions

Remove unused imports

Fix cleanup of image downloading useEffect (including cancellation)

Change ImageResizer component to a hook + cleanup useEffect

typing & typos

Fix fitImageContain & document

Improve injected code: pure functions where possible & documentation

Type navigation of CustomImageNavigator

Fix typing of ImageCropper (ref & style)

Fix post rebase

fix post rebase

lint

Add custom image (feature flagged) entry point in nft links panel

Fix rendering logic of NftLinksPanel (with sections & separator between)

update doc

Disable back gesture in the Error Screen

nft: add image/svg and image/svg+xml to mimeTypesMap for "image"

Fix custom image entry in NFT: should only be visible if there is an "image" uri

Remove fake url entrypoint

Fix layout of cropping UI

Revert bad change to NftViewer
Remove image/svg image/svg+xml from mimetypesmap

lint

Add wording for screen headers

Fix post rebase

Fix post rebase

Improve usage of the available area for the cropping UI


Implement native ImagePickerModule for Android (better than expo-image-picker)


Remove old dimensions logic


Remove unneeded loading of original image dimensions


Improve (Android) initial scaling of image in cropping UI (fills more space rather than fitting)


Remove customImage flag from debug screen


Fix post rebase
@ofreyssinet-ledger
Copy link
Contributor Author

ofreyssinet-ledger commented Sep 6, 2022

QA OK cf. https://ledgerhq.atlassian.net/browse/FAT-135?focusedCommentId=220655
and the bug mentioned by @alalmi-ledger on this comment is also fixed & mentioned as QA ok here https://ledgerhq.atlassian.net/browse/FAT-366

@ofreyssinet-ledger ofreyssinet-ledger merged commit a089100 into develop Sep 6, 2022
@ofreyssinet-ledger ofreyssinet-ledger deleted the feat/FAT-131-custom-image branch September 6, 2022 10:17
@ofreyssinet-ledger ofreyssinet-ledger changed the title FAT-131 - Custom image tool FAT-131 - Custom lock screen tool Jan 20, 2023
@ofreyssinet-ledger ofreyssinet-ledger changed the title FAT-131 - Custom lock screen tool FAT-131 - Stax Custom lock screen tool Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common Has changes in live-common ledgerjs Has changes in the ledgerjs open source libs mobile Has changes in LLM translations Translation files have been touched
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants