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.

Sign up for GitHub

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

Jump to bottom

chore: remove unnecessary dark classes #255

Merged
merged 1 commit into from
Jul 13, 2024
Merged

chore: remove unnecessary dark classes #255

merged 1 commit into from
Jul 13, 2024
+14 −14

Conversation

im-adithya
Copy link
Member

title

@im-adithya im-adithya requested a review from reneaaron July 11, 2024 07:35
rolznz
rolznz reviewed Jul 12, 2024
View reviewed changes
frontend/src/components/TransactionItem.tsx
@@ -67,14 +67,14 @@ function TransactionItem({ tx }: Props) {
className={cn(
"w-6 h-6 md:w-8 md:h-8",
type === "outgoing"
? "text-orange-400 dark:text-amber-600 stroke-orange-400 dark:stroke-amber-600"
: "text-green-500 dark:text-emerald-500 stroke-green-400 dark:stroke-emerald-500"
? "stroke-orange-400 dark:stroke-amber-600"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know what happened, I thought I already commented this but I think we should also not hardcode sent and received colors. Is there a good way we can do this? CC @reneaaron

Copy link
Contributor

Choose a reason for hiding this comment

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

We could of course introduce new variables, but since it's the only placed used right now I would be good to keep it as is.

As soon as we need those colors in other places as well I'd probably introduce separate theme variables for that purpose. Does that make sense for you?

Copy link
Contributor

Choose a reason for hiding this comment

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

@reneaaron sounds good!

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 and @rolznz I opened a similar PR to clean up wrong CSS classnames in the extension repo, you have commented there 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

🤦‍♂️ thanks @im-adithya !

rolznz
rolznz approved these changes Jul 13, 2024
View reviewed changes
Copy link
Contributor

@rolznz rolznz left a comment

Choose a reason for hiding this comment

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

utACK

@rolznz rolznz merged commit 1380934 into master Jul 13, 2024
8 checks passed
@rolznz rolznz deleted the task-dark-classes branch July 13, 2024 05:02
rolznz added a commit that referenced this pull request Jul 14, 2024
@rolznz @im-adithya
@reneaaron @bumi
Feat: permissions revamp v2 (#273)
e297251 
* feat: revamp permissions component

* chore: changes

* chore: changes

* chore: add view mode for show app screen

* chore: further changes

* feat: new illustration for linking account (#254)

* feat: new illustration for linking account

* fix: update paths

* fix: icon props (#256)

* fix: use date from frontend

* chore: add expiryselect component

* chore: further changes

* chore: further changes

* chore: add date-fns for date picker

* chore: further changes

* chore: further changes

* fix: add central LDK gossip node to help gossip new public channels (#262)

* fix: make dialog responsive (#258)

* chore: further changes

* typo

* chore: spacing issues

* chore: use scopes from capabilities

* chore: change scope type descriptions

* chore: styling fixes

* chore: fix typings

* chore: budget renewal component

* fix: LDK mark channel as inactive and show error if counterparty forwarding info missing (#267)

fix: mark channel as inactive and show error if counterparty forwarding info missing

* chore: remove unnecessary dark classes (#255)

* fix: links to open first channel in sidebar and onboarding checklist (#268)

* feat: improve migrate node UI (#269)

* fix: migrate node copy (#270)

* fix: stop nostr when app is shutdown and use context to stop lnclient (#271)

* fix: permissions revamp WIP

* feat: basic isolated apps UI

* fix: new app connection, edit app connection, deep linking

---------

Co-authored-by: im-adithya <[email protected]>
Co-authored-by: René Aaron <[email protected]>
Co-authored-by: Michael Bumann <[email protected]>
rolznz added a commit that referenced this pull request Jul 19, 2024
@rolznz @im-adithya
@reneaaron @bumi
Feat: dynamic budgets (#226)
1391119 
* feat: add transactions table

* feat: add transactions service with makeinvoice method

* feat: use internal transactions WIP

* feat: check unsettled transactions

* feat: use transactions service in nip47 package WIP

* feat: use transactions service in NIP-47 handlers

* fix: transaction list colors

* fix: tests

* chore: remove old payments table

* fix: return budget usage as sats

* feat: use internal transactions table for keysend payments

* feat: consume nwc_payment_received event in transaction service (WIP)

* feat: update existing transaction from nwc_payment_received event in transactions service

* feat: update existing transaction from nwc_payment_sent event in transactions service

* feat: handle async payment failed events in transactions service

* fix: tests

* feat: correctly implement NIP-47 NOT_FOUND error code

* feat: intercept self payments

* feat: isolated balance and visibility

* feat: validate keysend payment does not exceed app internal balance

* feat: budget check in transactions service, correctly pass payment errors to NIP-47 response

also reduce query duplication

* fix: order transactions when looking up transaction

* feat: add fee reserves to unsettled outgoing transactions

* chore: rename app permissions max amount field

* chore: rename transaction amount values to be clearly millisats

* chore: merge balance type and visibility into isolated property on apps table

* chore: move duplicated permission check from nip-47 controllers to handler

* fix: app name in transaction list

* Feat: permissions revamp v2 (#273)

* feat: revamp permissions component

* chore: changes

* chore: changes

* chore: add view mode for show app screen

* chore: further changes

* feat: new illustration for linking account (#254)

* feat: new illustration for linking account

* fix: update paths

* fix: icon props (#256)

* fix: use date from frontend

* chore: add expiryselect component

* chore: further changes

* chore: further changes

* chore: add date-fns for date picker

* chore: further changes

* chore: further changes

* fix: add central LDK gossip node to help gossip new public channels (#262)

* fix: make dialog responsive (#258)

* chore: further changes

* typo

* chore: spacing issues

* chore: use scopes from capabilities

* chore: change scope type descriptions

* chore: styling fixes

* chore: fix typings

* chore: budget renewal component

* fix: LDK mark channel as inactive and show error if counterparty forwarding info missing (#267)

fix: mark channel as inactive and show error if counterparty forwarding info missing

* chore: remove unnecessary dark classes (#255)

* fix: links to open first channel in sidebar and onboarding checklist (#268)

* feat: improve migrate node UI (#269)

* fix: migrate node copy (#270)

* fix: stop nostr when app is shutdown and use context to stop lnclient (#271)

* fix: permissions revamp WIP

* feat: basic isolated apps UI

* fix: new app connection, edit app connection, deep linking

---------

Co-authored-by: im-adithya <[email protected]>
Co-authored-by: René Aaron <[email protected]>
Co-authored-by: Michael Bumann <[email protected]>

* fix: make transactions table ID autoincrement

* fix: do not send unrelated notifications to isolated apps

* fix: nip-47 notifications not receiving updated transaction state

* fix: migrate existing tables to have autoincrementing primary keys (#274)

* fix: migrate existing tables to have autoincrementing primary keys

* fix: recreate request and response event tables after apps

* fix: remove null from request_events app_id

* fix: do not crash when reloading app created page

* chore: update pragma commands, add busy_timeout

* fix: remove unnecessary migration of user_configs table

* chore: update comment on autoincrement migration

* feat: sqlite database config improvements

* fix: autoincrement migration to delete unlinked app permissions

* chore: update tests for dynamic budgets (WIP)

* fix: get balance tests

* chore: add extra event handler tests

* chore: add extra event handler test

* chore: add extra multi_pay_invoice tests

* chore: transactions service tests (WIP)

* chore: add transactions service payment tests

* chore: add tests for self payments

* fix: notifications tests

* chore: add notifications tests for transactions service

* chore: add fee reserve tests for transactions service

* chore: add list transactions tests

* chore: add keysend tests for transactions service

* feat: subscribe for payments and invoices (#281)

* fix: db transaction should be passed as pointer

* feat(lnd): subscribe for payments and invoices

* chore: rearrange check for settled

* chore: remove TODO

* chore: retry on error and add select

* chore: publish payment failed event

* chore: use json logging

* feat: add lnd notification types

* chore: remove sleep

* fix: incorrect key on transaction list items

* fix: test

* fix: disable isolated app type on non-supported backends

---------

Co-authored-by: im-adithya <[email protected]>
Co-authored-by: René Aaron <[email protected]>
Co-authored-by: Michael Bumann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rolznz rolznz rolznz approved these changes

@reneaaron reneaaron Awaiting requested review from reneaaron

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@im-adithya @rolznz @reneaaron