Skip to content

Commit

Permalink
fix(table): default sort compare logic for date strings (#6153)
Browse files Browse the repository at this point in the history
  • Loading branch information
jacobmllr95 authored Dec 7, 2020
1 parent 5bf6733 commit 3696a1f
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 22 deletions.
4 changes: 2 additions & 2 deletions src/components/alert/alert.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,11 @@ export const BAlert = /*#__PURE__*/ Vue.extend({
countDown(newValue) {
this.clearCountDownInterval()
const show = this[MODEL_PROP_NAME]
// Ignore if `show` transitions to a boolean value
if (isNumeric(show)) {
// Ignore if this.show transitions to a boolean value.
this.$emit(EVENT_NAME_DISMISS_COUNT_DOWN, newValue)
// Update the v-model if needed
if (show !== newValue) {
// Update the v-model if needed
this.$emit(MODEL_EVENT_NAME, newValue)
}
if (newValue > 0) {
Expand Down
5 changes: 2 additions & 3 deletions src/components/avatar/avatar.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import {
PROP_TYPE_STRING
} from '../../constants/props'
import { SLOT_NAME_BADGE } from '../../constants/slots'
import { RX_NUMBER } from '../../constants/regex'
import { isNumber, isString } from '../../utils/inspect'
import { isNumber, isNumeric, isString } from '../../utils/inspect'
import { toFloat } from '../../utils/number'
import { omit, sortKeys } from '../../utils/object'
import { makeProp, makePropsConfigurable, pluckProps } from '../../utils/props'
Expand All @@ -33,7 +32,7 @@ const BADGE_FONT_SIZE_SCALE = FONT_SIZE_SCALE * 0.7

export const computeSize = value => {
// Parse to number when value is a float-like string
value = isString(value) && RX_NUMBER.test(value) ? toFloat(value, 0) : value
value = isString(value) && isNumeric(value) ? toFloat(value, 0) : value
// Convert all numbers to pixel values
return isNumber(value) ? `${value}px` : value || null
}
Expand Down
17 changes: 17 additions & 0 deletions src/components/table/helpers/default-sort-compare.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,23 @@ describe('table/helpers/default-sort-compare', () => {
expect(defaultSortCompare(date4, date4, options)).toBe(0)
})

it('sorts date strings correctly', async () => {
const date1 = { a: new Date(2020, 1, 1).toISOString() }
const date2 = { a: new Date(1999, 11, 31).toISOString() }
const date3 = { a: new Date(1999, 1, 1).toISOString() }
const date4 = { a: new Date(1999, 1, 1, 12, 12, 12, 12).toISOString() }
const options = { sortBy: 'a' }

expect(defaultSortCompare(date1, date2, options)).toBe(1)
expect(defaultSortCompare(date1, date1, options)).toBe(0)
expect(defaultSortCompare(date2, date1, options)).toBe(-1)
expect(defaultSortCompare(date2, date3, options)).toBe(1)
expect(defaultSortCompare(date3, date2, options)).toBe(-1)
expect(defaultSortCompare(date3, date4, options)).toBe(-1)
expect(defaultSortCompare(date4, date3, options)).toBe(1)
expect(defaultSortCompare(date4, date4, options)).toBe(0)
})

it('sorts strings correctly', async () => {
const options = { sortBy: 'a' }

Expand Down
4 changes: 2 additions & 2 deletions src/utils/inspect.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { RX_NUMBER } from '../constants/regex'
import { File } from '../constants/safe-types'

// --- Convenience inspection utilities ---
Expand Down Expand Up @@ -26,8 +27,7 @@ export const isString = value => toType(value) === 'string'

export const isNumber = value => toType(value) === 'number'

// Is a value number like (i.e. a number or a number as string)
export const isNumeric = value => !isNaN(parseInt(value, 10))
export const isNumeric = value => RX_NUMBER.test(String(value))

export const isPrimitive = value => isBoolean(value) || isString(value) || isNumber(value)

Expand Down
49 changes: 34 additions & 15 deletions src/utils/inspect.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
isBoolean,
isString,
isNumber,
isNumeric,
isPrimitive,
isDate,
isEvent,
Expand All @@ -17,7 +18,7 @@ import {
} from './inspect'

describe('utils/inspect', () => {
it('toType', async () => {
it('toType()', async () => {
expect(toType(123)).toEqual('number')
expect(toType('123')).toEqual('string')
expect(toType(true)).toEqual('boolean')
Expand All @@ -31,7 +32,7 @@ describe('utils/inspect', () => {
expect(toType(null)).toEqual('object')
})

it('toRawType', async () => {
it('toRawType()', async () => {
expect(toRawType(123)).toEqual('Number')
expect(toRawType('123')).toEqual('String')
expect(toRawType(true)).toEqual('Boolean')
Expand All @@ -45,7 +46,7 @@ describe('utils/inspect', () => {
expect(toRawType(null)).toEqual('Null')
})

it('toRawTypeLC', async () => {
it('toRawTypeLC()', async () => {
expect(toRawTypeLC(123)).toEqual('number')
expect(toRawTypeLC('123')).toEqual('string')
expect(toRawTypeLC(true)).toEqual('boolean')
Expand All @@ -59,7 +60,7 @@ describe('utils/inspect', () => {
expect(toRawTypeLC(null)).toEqual('null')
})

it('isUndefined', async () => {
it('isUndefined()', async () => {
expect(isUndefined(123)).toEqual(false)
expect(isUndefined('123')).toEqual(false)
expect(isUndefined(true)).toEqual(false)
Expand All @@ -73,7 +74,7 @@ describe('utils/inspect', () => {
expect(isUndefined(null)).toEqual(false)
})

it('isNull', async () => {
it('isNull()', async () => {
expect(isNull(123)).toEqual(false)
expect(isNull('123')).toEqual(false)
expect(isNull(true)).toEqual(false)
Expand All @@ -87,7 +88,7 @@ describe('utils/inspect', () => {
expect(isNull(null)).toEqual(true)
})

it('isUndefinedOrNull', async () => {
it('isUndefinedOrNull()', async () => {
expect(isUndefinedOrNull(123)).toEqual(false)
expect(isUndefinedOrNull('123')).toEqual(false)
expect(isUndefinedOrNull(true)).toEqual(false)
Expand All @@ -101,7 +102,7 @@ describe('utils/inspect', () => {
expect(isUndefinedOrNull(null)).toEqual(true)
})

it('isFunction', async () => {
it('isFunction()', async () => {
expect(isFunction(123)).toEqual(false)
expect(isFunction('123')).toEqual(false)
expect(isFunction(true)).toEqual(false)
Expand All @@ -115,7 +116,7 @@ describe('utils/inspect', () => {
expect(isFunction(null)).toEqual(false)
})

it('isBoolean', async () => {
it('isBoolean()', async () => {
expect(isBoolean(123)).toEqual(false)
expect(isBoolean('123')).toEqual(false)
expect(isBoolean(true)).toEqual(true)
Expand All @@ -129,7 +130,7 @@ describe('utils/inspect', () => {
expect(isBoolean(null)).toEqual(false)
})

it('isString', async () => {
it('isString()', async () => {
expect(isString(123)).toEqual(false)
expect(isString('123')).toEqual(true)
expect(isString(true)).toEqual(false)
Expand All @@ -143,8 +144,9 @@ describe('utils/inspect', () => {
expect(isString(null)).toEqual(false)
})

it('isNumber', async () => {
it('isNumber()', async () => {
expect(isNumber(123)).toEqual(true)
expect(isNumber(123.5)).toEqual(true)
expect(isNumber('123')).toEqual(false)
expect(isNumber(true)).toEqual(false)
expect(isNumber({})).toEqual(false)
Expand All @@ -157,7 +159,24 @@ describe('utils/inspect', () => {
expect(isNumber(null)).toEqual(false)
})

it('isPrimitive', async () => {
it('isNumeric()', async () => {
expect(isNumeric(123)).toEqual(true)
expect(isNumeric(123.5)).toEqual(true)
expect(isNumeric('123')).toEqual(true)
expect(isNumeric('123.5')).toEqual(true)
expect(isNumeric('123,5')).toEqual(false)
expect(isNumeric(true)).toEqual(false)
expect(isNumeric({})).toEqual(false)
expect(isNumeric([])).toEqual(false)
expect(isNumeric(/abc/)).toEqual(false)
expect(isNumeric(() => {})).toEqual(false)
expect(isNumeric(Date)).toEqual(false)
expect(isNumeric(new Date())).toEqual(false)
expect(isNumeric(undefined)).toEqual(false)
expect(isNumeric(null)).toEqual(false)
})

it('isPrimitive()', async () => {
expect(isPrimitive(123)).toEqual(true)
expect(isPrimitive('123')).toEqual(true)
expect(isPrimitive(true)).toEqual(true)
Expand All @@ -171,7 +190,7 @@ describe('utils/inspect', () => {
expect(isPrimitive(null)).toEqual(false)
})

it('isDate', async () => {
it('isDate()', async () => {
expect(isDate(123)).toEqual(false)
expect(isDate('123')).toEqual(false)
expect(isDate(true)).toEqual(false)
Expand All @@ -185,7 +204,7 @@ describe('utils/inspect', () => {
expect(isDate(null)).toEqual(false)
})

it('isEvent', async () => {
it('isEvent()', async () => {
expect(isEvent(123)).toEqual(false)
expect(isEvent('123')).toEqual(false)
expect(isEvent(true)).toEqual(false)
Expand All @@ -201,7 +220,7 @@ describe('utils/inspect', () => {
expect(isEvent(null)).toEqual(false)
})

it('isRegExp', async () => {
it('isRegExp()', async () => {
expect(isRegExp(123)).toEqual(false)
expect(isRegExp('123')).toEqual(false)
expect(isRegExp(true)).toEqual(false)
Expand All @@ -215,7 +234,7 @@ describe('utils/inspect', () => {
expect(isRegExp(null)).toEqual(false)
})

it('isPromise', async () => {
it('isPromise()', async () => {
expect(isPromise(123)).toEqual(false)
expect(isPromise('123')).toEqual(false)
expect(isPromise(true)).toEqual(false)
Expand Down

0 comments on commit 3696a1f

Please sign in to comment.