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

create representations for content instances #70

Merged
merged 2 commits into from
Nov 7, 2023

Conversation

anthonie-kramer
Copy link
Contributor

@anthonie-kramer anthonie-kramer commented Nov 3, 2023


This change is Reviewable

@anthonie-kramer
Copy link
Contributor Author

I'm publishing function-SpaceType with the LayoutStrategies change so that it will work with InteriorPartitions udpate

Copy link
Member

@andrewheumann andrewheumann left a comment

Choose a reason for hiding this comment

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

Reviewed 35 of 35 files at r1, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained, 1 unresolved discussion (waiting on @anthonie-kramer)


LayoutFunctions/LayoutFunctionCommon/LayoutStrategies.cs line 58 at r1 (raw file):

            layoutInstantiated.Instance = componentDefinition.Instantiate(ContentConfiguration.AnchorsFromRect(rectangle.TransformedPolygon(xform)));

            // Ensure that all content items have representations

Can we leave a TODO to remove this when the corresponding elements PR is merged?

Copy link
Member

@andrewheumann andrewheumann left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 change requests, 0 of 1 approvals obtained, 1 unresolved discussion (waiting on @anthonie-kramer)


LayoutFunctions/LayoutFunctionCommon/LayoutStrategies.cs line 58 at r1 (raw file):

Previously, andrewheumann (Andrew Heumann) wrote…

Can we leave a TODO to remove this when the corresponding elements PR is merged?

now that this is merged, can we remove this code and point the function to the correct elements alpha?

Copy link
Contributor Author

@anthonie-kramer anthonie-kramer left a comment

Choose a reason for hiding this comment

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

This PR is now just an update to the csproj since Elements can handle the representations now

Reviewable status: 1 change requests, 0 of 1 approvals obtained, 1 unresolved discussion (waiting on @andrewheumann)


LayoutFunctions/LayoutFunctionCommon/LayoutStrategies.cs line 58 at r1 (raw file):

Previously, andrewheumann (Andrew Heumann) wrote…

now that this is merged, can we remove this code and point the function to the correct elements alpha?

Tested and removed

Copy link
Member

@andrewheumann andrewheumann left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained, all discussions resolved

@andrewheumann andrewheumann merged commit e7cdce0 into master Nov 7, 2023
1 check failed
@andrewheumann andrewheumann deleted the fix-content-representations branch November 7, 2023 18:33
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