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

fix: disable the checkbox for budget control during payments #2731

Merged
merged 18 commits into from
Sep 15, 2023

Conversation

volfiros
Copy link
Contributor

@volfiros volfiros commented Sep 6, 2023

Describe the changes you have made in this PR

Fixes - #2638

Type of change

  • fix: Bug fix (non-breaking change which fixes an issue)

Post changes

chrome_nE4WXh1aEy.mp4

How has this been tested?

Ran all the tests.

Checklist

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

@volfiros
Copy link
Contributor Author

volfiros commented Sep 6, 2023

@reneaaron I have made the changes for disabling the checkbox and to indicate that it is disabled I have blurred the label and the checkbox.
Please review

@im-adithya
Copy link
Member

You should disable it during the payment process, also I'd prefer reduced opacity to blur, thoughts @reneaaron?

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 for the PR! 🖐️

I've left some first feedback. 👌

@volfiros
Copy link
Contributor Author

volfiros commented Sep 7, 2023

@reneaaron @rolznz made the requested changes.
@stackingsaunter which color should I use for disabled inputs or should I decrease the opacity of the label?
Should I follow the color scheme mentioned in #2540 ?

@volfiros volfiros changed the title Disable the checkbox for budget control during payments fix: disable the checkbox for budget control during payments Sep 7, 2023
@volfiros volfiros requested a review from reneaaron September 7, 2023 15:08
@volfiros
Copy link
Contributor Author

volfiros commented Sep 9, 2023

@rolznz done the mentioned change, can u review it now.
Thanks!

@volfiros volfiros closed this Sep 9, 2023
@volfiros volfiros reopened this Sep 9, 2023
@rolznz
Copy link
Contributor

rolznz commented Sep 14, 2023

@Rithvik-padma can you also disable the budget control for keysend payments?

@rolznz
Copy link
Contributor

rolznz commented Sep 14, 2023

Unfortunately I cannot update this branch. I believe it's because you did not create a branch for this PR, but made a PR from master.

@volfiros
Copy link
Contributor Author

@Rithvik-padma can you also disable the budget control for keysend payments?

Sure will do that!

@volfiros
Copy link
Contributor Author

Unfortunately I cannot update this branch. I believe it's because you did not create a branch for this PR, but made a PR from master.

Yes I am using the master branch for it.
I will make the necessary changes for keysend too and update the branch.
Thanks!

@volfiros
Copy link
Contributor Author

@rolznz I have disabled the budget control for the keysend payments.
Can you review it.
I did not change the color for the disabled inputs.
@stackingsaunter should I use any color for it or leave it how it is now?
Thanks!

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.

tACK

I don't believe any styling changes are necessary.

@rolznz rolznz merged commit 8da1278 into getAlby:master Sep 15, 2023
@rolznz
Copy link
Contributor

rolznz commented Sep 15, 2023

Thanks for your work, @Rithvik-padma !

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.

4 participants