Table Refactor #520
Table Refactor #520
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| <!-- Arrows --> | ||
| <div class="paginator-arrows space-x-2"> | ||
| <button class="btn-icon {buttons}" on:click={onPrev} disabled={offset === 0}>←</button> | ||
| <button class="btn-icon {buttons}" on:click={onNext} disabled={(offset + 1) * limit >= size}>→</button> | ||
| <button class="btn-icon {buttons}" on:click={onPrev} disabled={settings.offset === 0}>←</button> |
There was a problem hiding this comment.
To represent what the action is, given the unicode? That opens us up to i18n issues, so perhaps we just make those straight labels like Dialog buttons?
There was a problem hiding this comment.
Yeah, they need to be screenreader accessible.
There was a problem hiding this comment.
I'll handle this as part of the updates here:
#465
src/lib/components/Table/utils.ts
Outdated
| // Cell Formatters --- | ||
|
|
||
| /** Wrap object key value with HTML tags. */ | ||
| export function tableCellFormatter(object: any[], key: string, openTag: string, closeTag: string): any[] { |
There was a problem hiding this comment.
I'm not so sure about this. You're opening yourself up to injections. I would prefer to see a solution based around document.createElement(tagName). I know that you're using this to also pass in classes, but it would be relatively trivial to pass global attributes as an optional argument. Change the type to unknown and remove the return cast to any[] and I can have a look at the typing later
There was a problem hiding this comment.
I've modified as follows:
- Now access
tagNameandclassesas params - Create a new entity using
createElement(tagName) - Appends the key value using
.innerHTML - Uses
setAttribute(classes)to append optional classes - Updates the object value using
.outerHTML, which returns as a string.
I think this will be better?
There was a problem hiding this comment.
Actually on second thought, this is not a good change to make. Svelte can't do anything with the document/DOM until the Mount event. This means all the table setup would have to occur during onMount which is slow as molasses.
This was just a bonus feature, so I think for now I'll just disable it until we can think this through a bit more.
There was a problem hiding this comment.
Good point about the onMount, hadn't considered that.
|
@ryceg I've knocked out all but the two items above. Just awaiting feedback on what's left. |
docs: Proofed the DataTables page
Submitting an early draft. Fixes #217