Table Preview Updates #1614
Table Preview Updates #1614
Conversation
|
@themadcreator does this overlap with #1603? Also instead of a reset button, what do you think about #1603 (comment) |
packages/table/preview/localStore.ts
Outdated
| * Demonstrates sample usage of the table component. | ||
| */ | ||
|
|
||
| import { defaults } from "lodash"; |
There was a problem hiding this comment.
I see lodash is only a dev dependency, but still, do we really need it for this one function? Would prefer not to pull it in if we've gone this long without it.
Alternatively, we could just cherry-pick the function we need, as mentioned in their docs:
// Cherry-pick methods for smaller browserify/rollup/webpack bundles.
var at = require('lodash/at');
var curryN = require('lodash/fp/curryN');There was a problem hiding this comment.
not important for preview, but i'll switch it
There was a problem hiding this comment.
when I switched to import * as defaults from "lodash/defaults" the types couldn't be resolved.
packages/table/preview/localStore.ts
Outdated
| this.storage = session ? sessionStorage : localStorage; | ||
| } | ||
|
|
||
| public getWithDefaults(defs?: T): T | {} { |
There was a problem hiding this comment.
Would prefer to call this defaults. defs means "definitions" to me.
There was a problem hiding this comment.
name already taken by the lodash method, but i'll come up w somethign
| return arr.indexOf(value) >= 0; | ||
| } | ||
|
|
||
| const DEFAULT_STATE: IMutableTableState = { |
There was a problem hiding this comment.
Can we define this below the IMutableTableState interface? Keeps the DEFAULTS definition closer to its usage in the constructor, and that order would just make more sense to me.
Delint. CommentsPreview: documentation | table |
Import sub modulePreview: documentation | table |
Separate mutable table into its own file.
Add session local storage for saving settings.
Add reset button to revert to defaults.