Skip to content

Clients - Vue Updates - #361

Merged
lakardion merged 11 commits into
mainfrom
feature/vue-to-date
Jan 7, 2025
Merged

Clients - Vue Updates#361
lakardion merged 11 commits into
mainfrom
feature/vue-to-date

Conversation

@lakardion

@lakardion lakardion commented Nov 8, 2024

Copy link
Copy Markdown
Contributor

Checklist

  • Move to SFCs (more concise and the current tendency)
  • Use Typescript for real in vue components
  • Move any JS to TS
  • Remove class-based styling, instead componentize and use tailwind
  • Erradicate use of margins. Use paddings instead. I think I'm gonna leave this for another PR

@oudeismetis
oudeismetis temporarily deployed to tn-spa-bootstrapper-pr-361 November 8, 2024 15:58 Inactive
@lakardion
lakardion temporarily deployed to tn-spa-bootstrapper-pr-361 November 11, 2024 22:26 Inactive
@lakardion
lakardion temporarily deployed to tn-spa-bootstrapper-pr-361 November 20, 2024 15:37 Inactive
@lakardion
lakardion temporarily deployed to tn-spa-bootstrapper-pr-361 November 21, 2024 09:21 Inactive
@lakardion
lakardion temporarily deployed to tn-spa-bootstrapper-pr-361 November 21, 2024 10:14 Inactive
@lakardion
lakardion temporarily deployed to tn-spa-bootstrapper-pr-361 November 21, 2024 10:18 Inactive
@lakardion
lakardion temporarily deployed to tn-spa-bootstrapper-pr-361 November 21, 2024 10:39 Inactive
@lakardion
lakardion temporarily deployed to tn-spa-bootstrapper-pr-361 November 21, 2024 10:53 Inactive
@lakardion
lakardion temporarily deployed to tn-spa-bootstrapper-pr-361 November 21, 2024 11:01 Inactive
@lakardion
lakardion temporarily deployed to tn-spa-bootstrapper-pr-361 November 21, 2024 11:01 Inactive
@oudeismetis

Copy link
Copy Markdown
Contributor

Good call on skipping the margin issue for now, this PR is already big enough
Will wait on Maya's opinion, but I'm taking a look now. Thank you for doing this 🙏

password: form.password.value ?? '',
confirmPassword: form.confirmPassword.value ?? '',
}
register(createAccountInput)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't form validation handle this and we should just be able to pass the form to the API call? Why do we need to manually extract each field?

@lakardion lakardion Nov 21, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a known bad interaction between tn-forms and typescript.
Even after validating the form, the type of the form value fields are <type> | undefined
Ideally, after form validation, the form.value should have the right required field types
I'm pushing for a good TS update to tn-forms but I don't think it is something that will come soon 😅

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yea the issue is that the actual definition of the form field values in the form is
<type>|undefined
Because value is an optional field
In addition to Dami's work I was thinking we could just add a type caster to avoid the issue

const loginForm = reactive(new LoginForm({}))
const forgotPasswordForm = reactive(new EmailForgotPasswordForm({}))
const resetPasswordForm = reactive(new ResetPasswordForm({}))
const registerForm = reactive(new AccountForm({}))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ahh need to fix this, we dont need to use the FormLevelValidator here and can just add it as it was

Welcome to {{ cookiecutter.project_name }}!
</h1>
<h1 class="text-4xl font-bold tracking-tight text-gray-900 sm:text-6xl">Welcome to vuets!</h1>
<p class="mt-6 text-lg leading-8 text-gray-600">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you accidentally replaced the cookiecutter bindings with your project

@paribaker paribaker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some of the bindings changed, can we also add the updated tn-models here with the .readonly

@lakardion
lakardion temporarily deployed to tn-spa-bootstrapper-pr-361 December 17, 2024 13:40 Inactive
@lakardion
lakardion requested a review from paribaker December 17, 2024 13:44
@lakardion

Copy link
Copy Markdown
Contributor Author

For tn models upgrade check #369

@oudeismetis

Copy link
Copy Markdown
Contributor

I assume we'll merge #369 first, and then rebase and merge this PR.

@lakardion
lakardion force-pushed the feature/vue-to-date branch from 08de420 to 855a49c Compare January 7, 2025 08:02
@lakardion
lakardion temporarily deployed to tn-spa-bootstrapper-pr-361 January 7, 2025 08:02 Inactive
@lakardion
lakardion dismissed paribaker’s stale review January 7, 2025 08:04

Fixed in another PR

@mayakeeley

Copy link
Copy Markdown
Contributor

@lakardion I like the changes you've made! The SFCs are a big improvement and I've been wanting to make the use of arrow functions consistent so thank you!! Looks like it's mostly been reviewed already so just adding this tiny note on the styling, I'm assuming it'll be picked up in the margin/padding PR though

image

@lakardion
lakardion merged commit 78eab59 into main Jan 7, 2025
@lakardion
lakardion deleted the feature/vue-to-date branch January 7, 2025 12:46
whusterj pushed a commit that referenced this pull request Aug 17, 2025
## Checklist
- [x] Move to SFCs (more concise and the current tendency)
- [x] Use Typescript for real in vue components
- [x] Move any JS to TS
- [x] Remove class-based styling, instead componentize and use tailwind
- ~Erradicate use of margins. Use paddings instead.~ I think I'm gonna
leave this for another PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants