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

List View viewzone API #103437

Closed
rebornix opened this issue Jul 27, 2020 · 4 comments
Closed

List View viewzone API #103437

rebornix opened this issue Jul 27, 2020 · 4 comments
Assignees
Labels
feature-request Request for new features or functionality list-widget List widget issues notebook
Milestone

Comments

@rebornix
Copy link
Member

While building side by side diff view for notebook (#94810), I ran into another challenge with dynamic cell heights.

At the beginning, List View supports dynamic items at creation/rendering time but it doesn't support item height change once it's rendered. Then we introduced updateElementHeight API to allow notebook cells grow its height (when there are outputs). Rendering notebooks side by side has more requirements: if a cell is modified/changed (for example, outputs are different), its scroll top in two editors should be the same, and its accumulated total height should be the same as well.

To solve this problem, we can either leverage updateElementHeight to update cell heights based on diff information (then cellHeight = inputHeight + outputsHeight + additionalHeight), or introduce a view zone API which is identical to Monaco's view zone. The API would allow you to preserve a whitespace area after an index:

interface IViewZone {
  afterIndex: number;
  heightInPx: number;
}

addZone(zone: IViewZone): string;
removeZone(id: string): void;
layoutZone(id: string): void;

The diff component can then use this API to adjust the scroll tops of cells without messing up with the layout info of cells. Diff information would be transparent to NotebookEditor or ListView.

@rebornix rebornix added list-widget List widget issues notebook feature-request Request for new features or functionality labels Jul 27, 2020
@rebornix rebornix added this to the Backlog milestone Jul 27, 2020
@rebornix
Copy link
Member Author

@roblourens is working on moving the height calculation logic into the List View other than doing all the math in notebook renderer. Maybe this can also be done by addViewZone and then update the height of the viewzone when DOM changes.

@joaomoreno
Copy link
Member

joaomoreno commented Jul 30, 2020

To solve this problem, we can either leverage updateElementHeight to update cell heights based on diff information (then cellHeight = inputHeight + outputsHeight + additionalHeight), or introduce a view zone API which is identical to Monaco's view zone. The API would allow you to preserve a whitespace area after an index:

Given the two approaches, the first one seems much better since it introduces no API and appears simple. Why do you seem to have a preference for a view zone API? And why can't a view zone feature set be built on top of the existing ListView API?

@joaomoreno
Copy link
Member

@rebornix Are we still planning on doing this?

@rebornix
Copy link
Member Author

rebornix commented Nov 2, 2020

@joaomoreno we probably don't need this anymore for now.

@rebornix rebornix closed this as completed Nov 2, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Dec 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality list-widget List widget issues notebook
Projects
None yet
Development

No branches or pull requests

3 participants