WIP: feat/tailwind-general-settings #2622

You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somewhere. The easiest to use is https://try.vikunja.io/.

This preview does not contain any changes made to the api, only the frontend.

Have a nice day!

Beep boop, I'm a bot.

Hi dpschen! Thank you for creating a PR! I've deployed the frontend changes of this PR on a preview environment under this URL: https://2622-feat-tailwind-general-settings--vikunja-frontend-preview.netlify.app You can use this url to view the changes live and test them out. You will need to manually connect this to an api running somewhere. The easiest to use is https://try.vikunja.io/. This preview does not contain any changes made to the api, only the frontend. Have a nice day! > Beep boop, I'm a bot.

konrad reviewed 2024-07-19 05:52:54 +00:00

frontend/src/views/user/settings/General.vue

						
				@ -5,3 +5,3 @@

						:loading="loading"

					>

						<div class="field">

						<div class="tw-grid tw-grid-cols-[1fr_2fr] tw-gap-3 tw-mb-3">

konrad commented

2024-07-19 05:52:52 +00:00

(I think I've commented that somewhere else already)

Can we make this into its own component?

(I think I've commented that somewhere else already) Can we make this into its own component?

👍 1

dpschen commented

2024-07-19 09:34:57 +00:00

I'm not 100% sure if this should be abstracted. widths of fields might change a lot depending on the context. If our app would be a government app with a lot of similar looking forms I would agree.

For our case I'm currently not yet convinced. Right now I think an abstraction just makes stuff more complex. E.g. the col width might be different depending on the use case. Obviously we could create a prop for this (pro component abstraction argument: the gap should probably always be identical).

If we want to move to radix-vue in general I think it would be best to wait and check if they don't offer some components for this (didn't check now).

I'm not 100% sure if this should be abstracted. widths of fields might change a lot depending on the context. If our app would be a government app with a lot of similar looking forms I would agree. For our case I'm currently not yet convinced. Right now I think an abstraction just makes stuff more complex. E.g. the col width might be different depending on the use case. Obviously we could create a prop for this (pro component abstraction argument: the gap should probably always be identical). If we want to move to radix-vue in general I think it would be best to wait and check if they don't offer some components for this (didn't check now).

konrad commented

2024-07-19 09:49:13 +00:00

I meant the field in general should be a component. We could still use inheritAttrs to allow changing the width etc depending on the context without having to add a bunch of props just for that.

I meant the field in general should be a component. We could still use `inheritAttrs` to allow changing the width etc depending on the context without having to add a bunch of props just for that.

dpschen commented

2024-07-19 10:06:17 +00:00

Do you mean 'in the General.vue component' or the saying 'in general'? :D

Because I realised that here in this component ther is only one div left from the original many field divs.

Do you mean 'in the `General.vue` component' or the saying 'in general'? :D Because I realised that here in this component ther is only one div left from the original many field divs.

konrad commented

2024-07-19 11:00:33 +00:00

"in general" 😅
The field class is used in a bunch of other places as well, I think it would make sense to abstract those into a component.

"in general" 😅 The field class is used in a bunch of other places as well, I think it would make sense to abstract those into a component.

dpschen commented

2024-11-30 18:09:35 +00:00

@konrad If we abstract the field as a general component I'd like to do that in a separate PR if that's okay.

konrad commented

2024-12-04 16:57:29 +00:00

sure!

🎉 1