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

Added function to allow total control over displayed value #4175

Merged

Conversation

Bond-Addict
Copy link
Contributor

This will partially negates the need to provide custom slots to manipulate the displayed value.

Description

For my use case I have ISO dates come in from my api. 2024/01/01 and I need them to display as 01/01/2024. By using the displayFormatFn I'm able to quickly handle formatting my dates to my desired format. This cleans up my template code as I no longer have to have a bunch of cell slots.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any feature but make things better)

…ially negates the need to provide custom slots to manipulate the displayed value.
{ key: 'name', displayFormatFn: () => 'Overridden string' },
{
key: 'date',
displayFormatFn: (date) => { return formatDate(date) },
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm okay with <template #cell(date)>{{ formatDate(date) }}</template>. Isn't it really gives you full control over how data is displayed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That does work, but if you have a bunch of columns that you need to manipulate it can clutter the template a bit. This allows that logic to stay in the script portion. If you wanted to do special styling, you would of course still need to use the slots. This just simplifies the method for overriding the displayed text value.

Copy link
Contributor

@m0ksem m0ksem Mar 6, 2024

Choose a reason for hiding this comment

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

I don't like having the ability to make the same thing with two different ways. I know we still provide this ability sometimes, but overall I think it is better when framework lets you only one way - the most customizable one.

I think you could make something like this if you don't want to junk your template.

<template #[`cell(${column})`]="{ value }" v-for="column in columns.filter((c) => c.displayFormatFn)">
  {{ displayFormatFn(value) }}
</template>

Copy link
Contributor

Choose a reason for hiding this comment

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

I personally use components like DataTablePhoneCell, DataTableEmailCell

const columns = defineVaDataTableColumns([
  { key: 'phone', type: DataTablePhoneCell },
  { key: 'email', type: DataTableEmailCell },
])

And this is in my wrapper component

<template #[`cell(${column})`]="cellBind" v-for="column in columns.filter((c) => c.type)">
  <component :is="c.type" v-bind="cellBind" />
</template>

I find this more useful than having formatting function, because you can also make cell link, add a small $ sign or whatever we might need in data table cells.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was going off of a similar behavior as https://ui.vuestic.dev/ui-elements/date-input#formatting. I think this way will
follow a familiar process and grant an easier development process, but ultimately its up to yall.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this should work?
Screenshot 2024-03-05 210925
image
Screenshot 2024-03-05 211048

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ask because its not working for me. Is it some weird mechanic similar to having to use

<template
						v-for="(_, name) in $slots"
						:key="name"
						v-slot:[name]="slotScope"
					>
						<slot
							:name="name"
							v-bind="slotScope"
						/>
					</template>

The example you provided below would be living inside my datatable wrapper component which is a child to most of my actual views.

	<template
						#[`cell(${column})`]="cellBind"
						v-for="column in columns.filter((c) => c.type)"
					>
						<component
							:is="column.type"
							v-bind="cellBind"
						/>
					</template>

So the flow is App.vue => RouterView => Main View => Table Wrapper View => DateValue.vue

I like your method for sure and I do see the upside of it, but I hope this does illustrate the extra layer of complexity needed to handle a simple display override.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@m0ksem for more complex displays, what would I need to do to get that option working for rendering a custom component based on the type like you suggested? Is it possible with the VaDataTable nested so low?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@m0ksem I now have a request to show a checkbox instead of the 1 or 0 value that comes back from our api. Your suggested method would fit this perfectly. So far I'm not able to utilize it though as it doesnt appear to work when its a child of a child. Is there some way around this? Alternatively, could we integrate <template #[`cell(${column})`]="cellBind" v-for="column in columns.filter((c) => c.type)" > <component :is="column.type" v-bind="cellBind" /> </template>
into the table component its self? It would be nice if you simply pass in the component type out of the box.

Copy link
Contributor

Choose a reason for hiding this comment

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

@m0ksem I now have a request to show a checkbox instead of the 1 or 0 value that comes back from our api. Your suggested method would fit this perfectly. So far I'm not able to utilize it though as it doesnt appear to work when its a child of a child. Is there some way around this? Alternatively, could we integrate <template #[`cell(${column})`]="cellBind" v-for="column in columns.filter((c) => c.type)" > <component :is="column.type" v-bind="cellBind" /> </template> into the table component its self? It would be nice if you simply pass in the component type out of the box.

We can add cellComponent to column.

@m0ksem
Copy link
Contributor

m0ksem commented Mar 6, 2024

Please, next time make a separate Story or add demo at the end. It is hard to see if there are any regressions in chromatic.
image

@m0ksem m0ksem merged commit ef64b4d into epicmaxco:develop Mar 6, 2024
4 checks passed
@Bond-Addict
Copy link
Contributor Author

Please, next time make a separate Story or add demo at the end. It is hard to see if there are any regressions in chromatic. image

Ah, okay. I'll make sure to do that. My apologies.

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.

None yet

2 participants