feat(a11y-table): Define a standalone table component #1634
Conversation
shadowbas
force-pushed
the
shadowbas/feat-accessible-table
branch
from
596aec9 to
972eb6a
Compare
| canvastablecontainer { | ||
| opacity: 0; | ||
| } | ||
|
|
There was a problem hiding this comment.
Would be better to instead get rid of anything canvas related in the code.
There was a problem hiding this comment.
- Remove this.canv and the canvas element from canvastable
src/app/app.component.scss
Outdated
| @@ -68,3 +72,10 @@ | |||
| width: 150px; | |||
| } | |||
|
|
|||
| .text-primary { | |||
| color: #01579b; | |||
There was a problem hiding this comment.
Move to a css property
| return Array.from(this.selectedRows).map(x => x.id); | ||
| } | ||
|
|
||
| updateRows() { |
There was a problem hiding this comment.
This is where I normalize the sync messages and non-sync messages. It's a mess but the simplest I could thing of
src/app/app.component.ts
Outdated
| this.rows = (() => { | ||
| if (!this.canvastable?.rows?.rows?.length) return [] | ||
|
|
||
| if (Array.isArray(this.canvastable.rows.rows[0])) |
There was a problem hiding this comment.
This is for when the rows are the arrays where the first item is the messageid.
|
Initial thoughts
|
src/app/app.component.ts
Outdated
| async enrichRows() { | ||
| const { start, end } = this.renderedRange | ||
|
|
||
| this.rows = await Promise.all(mapOverIndexes(async (value) => { |
There was a problem hiding this comment.
This might cause a race condition. When renderedRange changes twice; it might happen that the first range change resolves later than the second range change.
- Find a way to merge resolved arrays in a way that will keep the latest and not overwrite fetched rows
Add table to the app component.
This removes the error where it complains about a property change after render.
…e canvastable dopaint
| @@ -0,0 +1,13 @@ | |||
| <cdk-virtual-scroll-viewport [itemSize]="firstRowHeight" class="email-viewport"> | |||
There was a problem hiding this comment.
- Rename to virtual-scroll-table.component
| import { debounceTime } from 'rxjs/operators'; | ||
|
|
||
| @Component({ | ||
| selector: 'app-accessible-table', |
There was a problem hiding this comment.
- Rename to
app-virtual-scroll-table
| <td class="checkbox-cell"> | ||
| <mat-checkbox | ||
| (click)="onCheckboxClick($event, item, index)" | ||
| [checked]="rowsSelectionModel.isSelected(item)" |
There was a problem hiding this comment.
- Remove event listener from the skeleton version of a row.
| (sortToggled)="updateSearch(true)"> | ||
| </canvastablecontainer> | ||
|
|
||
| <app-accessible-table |
There was a problem hiding this comment.
@castaway , at some point I think it makes sense to move this table definition into its own component.
I believe that time to be when we want to re-use this table in other places either because we want to have better routing for folders or for some other reason that requires re-use.
|
|
||
| } | ||
|
|
||
| .skeleton-bone { |
There was a problem hiding this comment.
Move skeleton styles to global styles.
|
|
||
| const result = (value / Math.pow(base, exponent)).toFixed(decimalPlaces); | ||
| return `${parseFloat(result)} ${suffixes[exponent]}`; | ||
| } |
There was a problem hiding this comment.
It appears we have a function in src/app/messagetable/messagetablerow.ts. The converting of a number that represents the amount of bytes to a human readable format is not specific to messagetable.
- Make a human-readable utility module and use that here and in the messagetablerow.ts.
| const selection = (this.selectionModel.isMultipleSelection() ? items : [items]) as T[]; | ||
| this.selectionModel.setSelection(...selection) | ||
| } | ||
| } |
There was a problem hiding this comment.
Angular allows for two way binding in the template. Here we wrap the angular Selection model and add the selected property to be used for binding.
| } | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
A SelectionModel that takes a predicate which is used to filter items selected using the .select(...items) method. This is handy to prevent selecting items that have not been loaded yet. This is relevant when doing a range select which contains items that have not been loaded into array yet.
| @HostListener('window:resize') | ||
| onWindowResize() { | ||
| this.resetWidth(); | ||
| } |
There was a problem hiding this comment.
Should this be here? When do we want to reset the widths?
| size: this.getRow(rowIndex).size, | ||
| }; | ||
| } | ||
|
|
There was a problem hiding this comment.
@castaway , could you please help with how to test the websocketsearchmaillist?
| } catch (e) { | ||
| // This shouldnt happen, it means something changed the stored | ||
| // data without updating the messagedisplay rows. | ||
| console.log('Tried to lookup ' + index + ' in searchIndex, isnt there! ' + e); | ||
| console.error('Tried to lookup ' + index + ' in searchIndex, isnt there! ' + e); |
There was a problem hiding this comment.
- Change
+ eto, efor more useful log in console
| seen: this.searchService.getDocData(this.getRowId(index)).seen, | ||
| }; | ||
|
|
||
| if (app.viewmode === 'conversations') { |
There was a problem hiding this comment.
- Add the conversation count also to the accessible table as it was in canvastable.
| // console.log(`${f}`); | ||
| // console.log(FS.stat(`${this.partitionsdir}/${f}`)); | ||
| }); | ||
| // }); |
There was a problem hiding this comment.
Removed the look as it does nothing but block the thread. Easy performance win
Add table to the app component.
Use j and k to scroll up and down in messages.Dropped the support for j and k scroll.