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

feat: currency switcher #1050

Merged
merged 5 commits into from
Jan 31, 2025
Merged

feat: currency switcher #1050

merged 5 commits into from
Jan 31, 2025

Conversation

pavanjoshi914
Copy link
Contributor

@pavanjoshi914 pavanjoshi914 commented Jan 31, 2025

image
image
image
image
image
image
image
image
image
image

@pavanjoshi914 pavanjoshi914 changed the title feat: language switcher feat: currency switcher Jan 31, 2025
@@ -66,6 +123,32 @@ function Settings() {
</SelectContent>
</Select>
</div>
<div className="grid gap-1.5">
<Label htmlFor="currency">Select Currency</Label>
Copy link
Contributor

@rolznz rolznz Jan 31, 2025

Choose a reason for hiding this comment

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

Suggested change
<Label htmlFor="currency">Select Currency</Label>
<Label htmlFor="currency">Fiat Currency</Label>

@@ -66,6 +123,32 @@ function Settings() {
</SelectContent>
</Select>
</div>
<div className="grid gap-1.5">
<Label htmlFor="currency">Select Currency</Label>
{loading ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this conditional needed? I removed it and reloaded and still seems to work ok

@@ -63,7 +63,7 @@ func (httpSvc *HttpService) RegisterSharedRoutes(e *echo.Echo) {
e.Use(middleware.SecureWithConfig(middleware.SecureConfig{
ContentTypeNosniff: "nosniff",
XFrameOptions: "DENY",
ContentSecurityPolicy: "default-src 'self'; img-src 'self' https://uploads.getalby-assets.com https://getalby.com; connect-src 'self' https://api.getalby.com https://getalby.com https://zapplanner.albylabs.com",
ContentSecurityPolicy: "default-src 'self'; img-src 'self' https://uploads.getalby-assets.com https://getalby.com; connect-src 'self' https://api.getalby.com https://getalby.com https://getalby.com https://zapplanner.albylabs.com",
Copy link
Contributor

Choose a reason for hiding this comment

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

getalby.com was added twice, I will remove it

@@ -91,6 +91,7 @@ func (httpSvc *HttpService) RegisterSharedRoutes(e *echo.Echo) {

e.GET("/api/info", httpSvc.infoHandler)
e.POST("/api/setup", httpSvc.setupHandler)
e.PATCH("/api/currency", httpSvc.setCurrencyHandler)
Copy link
Contributor

Choose a reason for hiding this comment

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

later we could have an /api/settings instead so we don't need to add multiple routes to manage different settings. But I added a TODO for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Edit: I will change it

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, this should go in the restricted group, it doesn't need to be a public endpoint

- simplify code
- change api endpoint to update settings rather than just currency
- add missing fiat value on receive invoice page
- allow passing classname to formatted fiat component
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

@rolznz
Copy link
Contributor

rolznz commented Jan 31, 2025

@pavanjoshi914 overall nice job! 💪

@rolznz rolznz merged commit 5140cfb into master Jan 31, 2025
10 checks passed
@rolznz rolznz deleted the currency-switcher branch January 31, 2025 11:46
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.

2 participants