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(table): add field data formatter prop #739

Merged
merged 4 commits into from Aug 4, 2017

Conversation

Projects
None yet
3 participants
@mosinve
Member

mosinve commented Jul 25, 2017

  1. Add new prop to field - formatter, to allow custom formatting via callback function
  2. Add api-url prop to pass to callback item provider function
  3. Move sortBy and sortDesc props to Props instead Data, to allow define custom sorting on mount
  4. Some refactor

TODO: modify docs, examples.

@mosinve mosinve added this to the v0.19.0 milestone Jul 25, 2017

@mosinve mosinve self-assigned this Jul 25, 2017

sortDesc: {
type: Boolean,
default: true
},

This comment has been minimized.

@tmorehouse

tmorehouse Jul 25, 2017

Member

You might get prop mutation errors on sortBy and sortDesc due to the listeners on head click that update these values.

You might need local copies of these in data (i.e. localSort and localDesc), and then use the new .sync modifiers on the props, and emit updated:sort-by and update:sort-desc events to update the users copy of the props. And then user watchers on the props to update the local copies.

And then in the _items computed prop, look at the local copies and not the prop versions.

This comment has been minimized.

@tmorehouse

tmorehouse Jul 25, 2017

Member

If you like, I can tweak the props/data to handle this.

@tmorehouse tmorehouse changed the title from WIP feat (table) add field data formatter prop to [WIP] feat(table): add field data formatter prop Jul 25, 2017

@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Jul 25, 2017

Just thinking you might want to break out the changes into separate PRs... one for the formatter function, and one for the new props for sorting, and one for the api-url

@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Jul 25, 2017

@mosinve I've created a separate PR #742 for making sort-by and sort-desc available as two-way props (via .sync modifier).

return toString(a[sortBy]).localeCompare(toString(b[sortBy]), undefined, {
numeric: true
});
return ((a[sortBy] < b[sortBy]) && -1) || ((a[sortBy] > b[sortBy]) && 1) || 0;

This comment has been minimized.

@tmorehouse
@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Jul 28, 2017

For your formatter function, you could have it default to a built in function that just returns the field data, and then the user can provide their own formatter if they want to do javascript based formatting.

Basically just pass the formatter function two parameters, the field key (or the field object), and the record data object. The formatter probably only needs the field key and not ht field object, as the user would most likely have a copy of the field objects in their app anyway.

@mosinve

This comment has been minimized.

Member

mosinve commented Jul 28, 2017

It was supposed as an alternative to slots, so if it not defined, then slots used.... In case if i make a default builtin func, then could be that slots probably will never work?
About params to func - i'll make it so

@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Jul 28, 2017

Hmmm...

The output of the built in function could be passed to the default content of the slots

>
<template v-for="(field,key) in fields">
<td v-if="hasFormatter(field)" :key="key" :class="tdClass(field, item, key)"
v-html="callFormatter(field, item, key)">

This comment has been minimized.

@tmorehouse

tmorehouse Jul 28, 2017

Member

This works well. You probably don't need to send the field object (as the user probably has a reference to it in their app.

But maybe you could pass field last, as most would probably only need key and the item record data.

This comment has been minimized.

@mosinve

mosinve Jul 28, 2017

Member

Yup, i understand, will remove field param, and check how it will work.

This comment has been minimized.

@tmorehouse

tmorehouse Jul 28, 2017

Member

Although it might be handy to have it still in there for some people, but as the 3rd argument.

This comment has been minimized.

@mosinve

mosinve Jul 28, 2017

Member

Yeah, ok, accepted))

mosinve added some commits Jul 25, 2017

1. Add new prop to field - formatter, to allow custom formatting via …
…callback function

2. Add api-url prop to pass to callback item provider function
3. Move sortBy and sortDesc props to Props instead Data, to allow define custom sorting on mount
4. Some refactor

@mosinve mosinve force-pushed the feat/table-field-formatter branch from a7f564f to fc32ccd Jul 28, 2017

this._providerSetLocal(items);
});
} else {
// Provider returned Array data
this._providerSetLocal(data);
}
},
hasFormatter(field) {
return field.formatter && ((typeof (field.formatter) === 'function') || (typeof (field.formatter) === 'string'));

This comment has been minimized.

@tmorehouse

tmorehouse Jul 28, 2017

Member

Ah, so you are allowing each field to have it's own formatter function (via the fields objects)

I originally thought you were going to have a global field Formatter function (a prop), which would return a format based on the key, item, and field passed to it.

Your solution allows for more flexibility. :)

This comment has been minimized.

@tmorehouse

tmorehouse Jul 28, 2017

Member

and what is the option for typeof (field.formatter) === 'string' handle?

This comment has been minimized.

@mosinve

mosinve Jul 29, 2017

Member

I check if supplied value is string or function, to determine if remote formatter is correctly provided.

This comment has been minimized.

@mosinve

mosinve Jul 29, 2017

Member

If provided formatter func declared in global scope and provided as Function to formatter prop, then we directly call it.

return field.formatter(item[key]);
if (field.formatter && (typeof (this.$parent[field.formatter]) === 'function'))
return this.$parent[field.formatter](item[key]);

This comment has been minimized.

@tmorehouse

tmorehouse Jul 28, 2017

Member

So what do these two lines handle?

This comment has been minimized.

@tmorehouse

tmorehouse Jul 28, 2017

Member

So, just making a guess... does this allow calling a method defined in the parent (when field.formatter is a string)?

This comment has been minimized.

@mosinve

mosinve Jul 29, 2017

Member

Yep, if formatter supplied as String, then we search for it in parent's methods, this solution have a potential bug, when we have table component in a some wrapper component, like b-table, but i don't find any more flexible solution.

This comment has been minimized.

@tmorehouse

tmorehouse Jul 29, 2017

Member

Could a user do this:

{
  data: {
    fields: {
      foo: { sortble: true, formatter this.fooFormatter },
      bar: { sortble: true, formatter this.barFormatter }
    }
  },
  methods: {
    fooFormatter(item, key, field) {
      return item.a + item.b;
    },
    barFormatter(item, key, field) {
      return item.a - item.b;
    },
 }
}

This comment has been minimized.

@tmorehouse

tmorehouse Jul 29, 2017

Member

I think the field formatter calling signature should include item, key, field as args.

That way a single formatter function could be used to handle multiple columns, based on the key. That way a user could provide either unique formatters, or a single generic formatter method depending on their preference.

This comment has been minimized.

@mosinve

mosinve Jul 29, 2017

Member

then we must pass these vars from child component.
and this declaration won't work, as parent functions aren't in children's scope. in case we want to use parent's methods, we must supply it as String, Function declaration only for global scoped which will be available to child

data: {
    fields: {
      foo: { sortble: true, formatter this.fooFormatter },
      bar: { sortble: true, formatter this.barFormatter }
    }
  },

This comment has been minimized.

@mosinve

mosinve Jul 29, 2017

Member

the main goal of this is to provide a way to modify single field by simple func... don;t like idea of all-in-one heavy variant

This comment has been minimized.

@tmorehouse

tmorehouse Jul 29, 2017

Member

But making the extra args available would make rendering for "virtual" columns (columns that don't have a value in items) be possible (i.e. a virtual column that is comprised of data from different fields within the item record)

Maybe make the formatter function use this signature:

formatter(value, item, key)

And the user can then ignore the extra item, key args if not needed.

Just thinking about adding a bit of flexibility to the formatter functions.

This comment has been minimized.

@mosinve

mosinve Jul 30, 2017

Member

Then we will have two ways of handling table data: slots and formatter callback.
Callbacks will give more reusability, as we can move formatter functions out to standalne module and use it as mixin at components and even as global mixin to app's Vueinstance,
while slots will allow to insert another components into table cell and also format field value and create virtual columns in case we don't need to reuse it at whole app.

@mosinve

This comment has been minimized.

Member

mosinve commented Jul 29, 2017

The real use case:

 const lastPostsFields = {
        id: {
            label: '#',
            sortable: 'id'
        },
        user: {
            label: 'Name',
            sortable: true,
            formatter: 'linkUser'
        },
        created_at: {
            label: 'Created at',
            sortable: true,
            class: 'text-center',
            formatter: 'formatDate'
        },
        type: {
            label: 'Type'
        },
        thread: {
            label: 'Title',
            formatter: 'linkThread'
        },
        text: {
            label: 'Text',
            formatter: 'formatPost'
        }
    }

methods in Vue instance:

      methods: {
            allcap (value) {
                return value.toUpperCase()
            },
            renderBalance (value) {
                if (value === null) {
                    return '0 rub'
                } else {
                    return value + ' rub'
                }
            },
            linkUser (value) {
                return '<a href="/user/' + value.id + '">' + value.name + '</a>'
            },
            linkThread (value) {
                return `<a href="qa/thread/${value.id}/details">${value.title}</a>`
            }
            billStatementType (value) {
                return value === 'expense'
                    ? '<span class="badge badge-warning">Expense</span>'
                    : '<span class="badge badge-success">Income</span>'
            },
            renderConfirmationFlag (value) {
                return value === true
                    ? '<span class="badge badge-info">Yes</span>'
                    : '<span class="badge badge-warning">No</span>'
            },
            loginType (value) {
                return value === true
                    ? '<span class="badge badge-info">OAuth</span>'
                    : '<span class="badge badge-default">Password</span>'
            },
            genderLabel (value) {
                return value === 'M'
                    ? '<span class="badge badge-warning"><i class="fa fa-male" aria-hidden="true"></i>Male</span>'
                    : '<span class="badge badge-info"><i class="fa fa-female" aria-hidden="true"></i>Female</span>'
            },
            formatNumber (value) {
                return accounting.formatNumber(value, 2)
            }
@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Jul 29, 2017

How would one use a formatter function for a virtual column? i.e. a column that appears in fieldsbut on in the raw items data?

@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Jul 29, 2017

ie.:

data: (
  fields: {
    name: { sortable: true },
    birthdate: { sortable: true, formatter: 'formatDate')
    age: { formatter: 'calcAge' }
  },
  items: {
    { name: 'bob', birthdate: { year: 1967, month: 12, day: 7 } },
    { name: 'jane', birthdate: { year: 1992, month: 4, day: 20 }  )
  }
},
methods: {
  formatDate(value) {
    return `${value.year}/${value.month}/${value.day}`;
  }
  calcAge(value, item) { // value is null/undefined in this case
    const d = item.birthdate;
    const bday = new Date(d.year, d.month-1, d.day);
    const diff = Date.now() - bday.getTime();
    return Math.abs((new Date(diff)).getUTCFullYear() - 1970);
  }
}

Without access to the item row data, it would be impossible to create a formatter for field age

@@ -147,6 +148,7 @@ Supported field properties:
| `thClass` | String or Array | Class name (or array of class names) to add to header/footer `<th>` cell
| `tdClass` | String or Array | Class name (or array of class names) to add to data `<td>` cells in the column
| `thStyle` | Object | JavaScript object representing CSS styles you would like to apply to the table field `<th>`
| `formatter` | String or Function | A formatter callback function, can be used instead of slots

This comment has been minimized.

@tmorehouse

tmorehouse Jul 29, 2017

Member

Can be used instead of slots, but only for fields that have a real value. Currently one cannot use this for creating data for a virtual column.

This comment has been minimized.

@mosinve

mosinve Jul 30, 2017

Member

Yep, and by design these featues, slots and formatter are mutually exclusive. Ie. if we define formatter at items array, then we cant use slots, and slots are used only if formatter is not defined.

This comment has been minimized.

@tmorehouse

tmorehouse Jul 30, 2017

Member

So that should be added to the documentation ;-)

@mosinve mosinve changed the title from [WIP] feat(table): add field data formatter prop to feat(table): add field data formatter prop Aug 4, 2017

@mosinve mosinve removed the Status: WIP label Aug 4, 2017

@mosinve mosinve merged commit 9da94a6 into master Aug 4, 2017

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@pi0

This comment has been minimized.

Member

pi0 commented Aug 11, 2017

Vue 2.4 SSR is incompatible with slots inside templates (or maybe because of v-if/v-else) and rendering raw <slot> tags like this:

image

@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Aug 11, 2017

Hmmmm....

@pi0

This comment has been minimized.

Member

pi0 commented Aug 11, 2017

Workaround: dc8d238 😆 New SSR optimizations for v-if are causing many bugs like this.

@mosinve mosinve deleted the feat/table-field-formatter branch Aug 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment