Skip to content
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

dash/datagrid2 #21182

Draft
wants to merge 50 commits into
base: master
Choose a base branch
from
Draft

dash/datagrid2 #21182

wants to merge 50 commits into from

Conversation

vazonik
Copy link
Member

@vazonik vazonik commented May 16, 2024

WiP

(ToDo list moved to Asana)


@highsoft-bot
Copy link
Collaborator

highsoft-bot commented May 16, 2024

File size comparison

Sizes for compiled+gzipped (bold) and compiled files.

master candidate difference
datagrid/datagrid.js 24.8 kB
76.9 kB
27.5 kB
88.6 kB
2759 B
11981 B

@highsoft-bot
Copy link
Collaborator

Visual test results - No difference found


if (value !== void 0) {
if (key === 'style') {
Object.assign(element.style, value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spread operator should be faster.

const element = document.createElement(tagName);

if (params) {
const paramsKeys = Object.keys(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can try to test Object.entries instead. It should be faster.

Copy link
Contributor

@karolkolodziej karolkolodziej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

  1. When writing api docs please stick to the convention:
    /**
     * Constructs a row in the data grid.
     *
     * @param dataGrid 
     * The data grid instance which the row belongs to.
     *
     * @param index 
     * The index of the row in the data table.
     */
  1. I would like to have some convention about the class lifecycle methods:
  • have "light" constructor that calls init
  • init should set some basic options and call load and/or render
  • update method is missing
  • reflow I don't like it, buf if we need it let's keep it
  • destroy is missing?

public viewport: DataGridTable;

/**
* The width ratio of the column.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please explain a bit more what it does and why?
As far as I see there is no way to set it right?

ts/DataGrid/DataGrid2/DataGridCell.ts Show resolved Hide resolved
/**
* Whether the row is destroyed.
*/
public destroyed: boolean = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

destroyed for me seems permanent and completed, I would use something like removed/hidden/notVisibleInViewport?

ts/DataGrid/DataGrid2/DataGridTable.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants