-
Notifications
You must be signed in to change notification settings - Fork 315
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
Drop Account#name, Account#starred and LLD to use Currency Settings' instead of Account#unit #6761
Conversation
The latest updates on your projects. Learn more about Vercel for Git βοΈ 5 Ignored Deployments
|
5a8ba05
to
03ef4fc
Compare
03ef4fc
to
e5d4d57
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still have around 150 files to check but I post this comments first so you can check them in the meantime
apps/ledger-live-desktop/src/renderer/components/AccountsList/AccountRow.tsx
Show resolved
Hide resolved
overflowStyles: React.CSSProperties = { | ||
const overflowStyles: React.CSSProperties = { | ||
textOverflow: "ellipsis", | ||
overflow: "hidden", | ||
whiteSpace: "nowrap", | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably should be moved outside the component
.../ledger-live-desktop/src/renderer/families/cardano/DelegationFlowModal/steps/StepSummary.tsx
Show resolved
Hide resolved
.../ledger-live-desktop/src/renderer/families/cardano/UndelegateFlowModal/steps/StepSummary.tsx
Show resolved
Hide resolved
apps/ledger-live-desktop/src/renderer/modals/SettingsAccount/AccountSettingRenderBody.tsx
Show resolved
Hide resolved
c0764f2
e5d4d57
to
c0764f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some more details but the rest LGTM
apps/ledger-live-desktop/src/renderer/screens/accounts/AccountRowItem/index.tsx
Show resolved
Hide resolved
apps/ledger-live-desktop/src/renderer/screens/accounts/AccountRowItem/index.tsx
Show resolved
Hide resolved
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Accounts/CurrencyRows.tsx
Show resolved
Hide resolved
c0764f2
to
74717c4
Compare
74717c4
to
e9c8c82
Compare
i'm moving this back to Draft because i think we need to ship #6791 with it too, so i think we'll merge the LLM part in this PR too and will need review on the extra parts that it brings there is notably one thing that we must fix globally to restore support of the |
the PR is replaced by #6796 |
β Checklist
npx changeset
was attached.π Description
This PR is a mashup of two PRs:
since these PR modify similar codebase, it will be simpler to get this PR tested and merged that solved all the conflicts.
Builds
β Context
π§ Checklist for the PR Reviewers