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

Implement allowances and sites pages UX/UI improvements #2617

Merged
merged 19 commits into from
Aug 31, 2023

Conversation

GabrielSoga
Copy link
Contributor

Describe the changes you have made in this PR

Budgets UX/UI improvements:

  • On popup view of a page that has not have a budget set, there should be a link to do it instead of empty budget bar (right now). Clicking it leads to Edit Preferences modal.
  • An icon button of that opens Edit Preferences modal should be in the corner of site box.
  • Modal has updated copy
  • New disconnect button in Edit Preferences modal.
  • New design for the Progress Bar
  • Changing "ACTIVE" badge to "BUDGET"
  • A revamp of website page views:
    • Logo, name and URL is on the left side
    • Site settings button that opens Edit Preferences modal
    • Displaying info about payments, budget and permissions (badges). If budget not set, showing a link

Link this PR to an issue

#2458

Type of change

(Remove other not matching type)

  • feat: New feature (non-breaking change which adds functionality)

How has this been tested?

Manual testing

Checklist

  • Self-review of changed code
  • Manual testing
  • Added automated tests where applicable
  • Update Docs & Guides
  • For UI-related changes
  • Darkmode
  • Responsive layout

@stackingsaunter
Copy link
Contributor

Thanks for the PR! I can't test it without a build though

@reneaaron
Copy link
Contributor

@fczuardi @GabrielSoga Thanks for this PR! 👍 Could you please have a look at the unit tests? Some of them seem to fail currently.

Planning to test / review this PR later today / start of next week.

@reneaaron reneaaron self-requested a review August 4, 2023 16:01
@reneaaron
Copy link
Contributor

@GabrielSoga Great job with this PR! 👍 From a functional perspective everything seemed to work as expected, I've pushed some minor improvements / fixes to this branch here:

https://github.com/getAlby/lightning-browser-extension/tree/budgets-new-ux
(for some reason I am not able to push to your branch, maybe you need to allow maintainers of getAlby to edit this PR?)

Will continue with the code review next week. Have a nice weekend! 😎

@fczuardi
Copy link
Contributor

fczuardi commented Aug 8, 2023

@fczuardi @GabrielSoga Thanks for this PR! 👍 Could you please have a look at the unit tests? Some of them seem to fail currently.

Planning to test / review this PR later today / start of next week.

Unit tests are updated.

@fczuardi
Copy link
Contributor

fczuardi commented Aug 8, 2023

Thanks for the PR! I can't test it without a build though

Builds are good now @stackingsaunter :)

@stackingsaunter
Copy link
Contributor

Tested, looks really nice, great job @GabrielSoga !!!

Thanks you!

Copy link
Contributor

@stackingsaunter stackingsaunter left a comment

Choose a reason for hiding this comment

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

tAck

@fczuardi
Copy link
Contributor

@reneaaron can we do something to help this branch? what are the next steps?

Copy link
Contributor

@reneaaron reneaaron left a comment

Choose a reason for hiding this comment

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

Thanks very much again for this PR, great work guys! 💯

I've added some minor fixes (responsive layout, color usage, created a Modal component to avoid code reuse, etc) and left a few comments.

Apart from the "division by zero" this looks good to merge from my side.

I've now also fixed those things.

tACK

src/extension/background-script/actions/allowances/get.ts Outdated Show resolved Hide resolved
src/extension/background-script/actions/allowances/list.ts Outdated Show resolved Hide resolved
src/i18n/locales/en/translation.json Outdated Show resolved Hide resolved
@fczuardi
Copy link
Contributor

nice cleanup! we like it :)

@reneaaron reneaaron merged commit 6bf96d4 into getAlby:master Aug 31, 2023
6 checks passed
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.

5 participants