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

feature: update workspace name #63

Merged
merged 1 commit into from
Jul 27, 2024
Merged

feature: update workspace name #63

merged 1 commit into from
Jul 27, 2024

Conversation

geclos
Copy link
Collaborator

@geclos geclos commented Jul 26, 2024

Implements first version of settings page with workspace name update

@geclos geclos force-pushed the feature/provider_apikeys branch from 33998e3 to 11ebfec Compare July 26, 2024 15:15
Implements first version of settings page with workspace name update
@geclos geclos force-pushed the feature/provider_apikeys branch from 11ebfec to 2cc5f52 Compare July 26, 2024 15:17
@geclos geclos requested review from andresgutgon and csansoon July 26, 2024 15:36
@geclos
Copy link
Collaborator Author

geclos commented Jul 27, 2024

🐮 👦🏼

@geclos geclos merged commit dda49f1 into main Jul 27, 2024
2 checks passed
@geclos geclos deleted the feature/provider_apikeys branch July 27, 2024 08:28
trailing: true,
})

// TODO: i18n
Copy link
Contributor

Choose a reason for hiding this comment

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

Really?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it adds little friction and adding i18n later is a massive pita

Copy link
Contributor

Choose a reason for hiding this comment

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

But this is an app for people who are prompting in English. Why would it need to be translated into other languages?

Adding i18n is always a pita. Less now than later but still maintaining translations is a burden that we might avoid in this app. My 2 cents


// TODO: i18n
return (
<div className='flex flex-col gap-4 max-w-[50%]'>
Copy link
Contributor

Choose a reason for hiding this comment

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

A simple form like this is easier with normal forms + revalidatePath. You could remove a lot of the code done here

Copy link
Contributor

Choose a reason for hiding this comment

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

No necessary to debounce because you don't manage input value. You let uncontrolled and on form submit action goes to the server like a plain old rails app

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

let's talk offline

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