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

Split layout and draw for slots #524

Merged
merged 8 commits into from
Feb 14, 2025
Merged

Split layout and draw for slots #524

merged 8 commits into from
Feb 14, 2025

Conversation

huchenlei
Copy link
Member

@huchenlei huchenlei commented Feb 13, 2025

Replaces #510

This PR handles a smaller scope that only rewrites the layout/draw of node slots.
Layout objects are stored directly on original slot refs as _layoutElement.

src/NodeSlot.ts Outdated Show resolved Hide resolved
src/interfaces.ts Outdated Show resolved Hide resolved
src/interfaces.ts Outdated Show resolved Hide resolved
src/interfaces.ts Outdated Show resolved Hide resolved
src/interfaces.ts Outdated Show resolved Hide resolved
Comment on lines 1 to 26
import { Point, ReadOnlyRect } from "@/interfaces"

export class LayoutElement<T> {
public readonly value: T
public readonly boundingRect: ReadOnlyRect
public readonly highlight?: boolean
public readonly invalid?: boolean

constructor(o: {
value: T
boundingRect: ReadOnlyRect
highlight?: boolean
invalid?: boolean
}) {
Object.assign(this, o)
this.value = o.value
this.boundingRect = o.boundingRect
}

get center(): Point {
return [
this.boundingRect[0] + this.boundingRect[2] / 2,
this.boundingRect[1] + this.boundingRect[3] / 2,
]
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a bit confused by this class - on every layout:

  • Convert slot to class if necessary
  • Create new instance of LayoutElement
  • Copy all properties of slot to layout element
  • Copy original slot itself to layout element

If we want to create layout objects and attach them to data objects, we should probably do this once - during data object construction or lazy loaded. When updates are needed, update their values.

The benefit of replacing immutable structs in layout code doesn't really apply to JS (yet). As far as I know there's not actually proper tooling for handling this kind of thing (records are stage 2 apparently?).

Copy link
Member Author

Choose a reason for hiding this comment

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

It probably won't happen in this PR but the plan is to only update layout object when actual layout happens, i.e. when node is getting resized

@huchenlei huchenlei merged commit ce44cea into master Feb 14, 2025
4 checks passed
@huchenlei huchenlei deleted the slots_layout branch February 14, 2025 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants