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

Added Structured Table Type #353

Closed
wants to merge 3 commits into from
Closed

Added Structured Table Type #353

wants to merge 3 commits into from

Conversation

mybabysexy
Copy link

Implement #101

@github-actions
Copy link

github-actions bot commented Aug 31, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@mybabysexy
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@dvesh3 dvesh3 added this to the 1.9.0 milestone Sep 12, 2023
Copy link
Member

@fashxp fashxp left a comment

Choose a reason for hiding this comment

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

Thx very much. We would need some tweaks here and there though.

*/
public function evaluateReturnType(string $inputType, int $index = null): string
{
return TransformationDataTypeService::DEFAULT_ARRAY;
Copy link
Member

Choose a reason for hiding this comment

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

we would need to introduce there an extra type which just allows assignment to structured table fields, like we have it for quantity value, gallery etc.

'languages'
'languages',
'table',
'structuredTable'
Copy link
Member

Choose a reason for hiding this comment

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

should be moved to an extra type...

if (empty($inputData)) {
return "";
}
return (new StructuredTable($inputData))->getHtmlTable(
Copy link
Member

Choose a reason for hiding this comment

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

Why return a html table here?
Shouldn't be that in generateResultPreview, as for example here

'key' => $value,
'label' => $label,
];
}, array_keys($inputData), array_keys(array_values($inputData)[0])),
Copy link
Member

Choose a reason for hiding this comment

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

$inputData might be empty/null ... will result in errors at this place.

@fashxp
Copy link
Member

fashxp commented Feb 27, 2024

@mybabysexy 🏓 did you already have a chance to take a look at my comments?

@fashxp
Copy link
Member

fashxp commented Apr 30, 2024

@mybabysexy 🏓

@fashxp
Copy link
Member

fashxp commented Jun 10, 2024

@mybabysexy 🏓 any chance we could continue the discussion here?

@mybabysexy
Copy link
Author

@mybabysexy 🏓 any chance we could continue the discussion here?

hi, sorry I'm no longer working with Pimcore or PHP so I don't think that I can continue with this PR

@fashxp fashxp removed this from the 1.9.0 milestone Jun 10, 2024
@fashxp
Copy link
Member

fashxp commented Jun 10, 2024

@mybabysexy 🏓 any chance we could continue the discussion here?

hi, sorry I'm no longer working with Pimcore or PHP so I don't think that I can continue with this PR

thx for the information.

@fashxp
Copy link
Member

fashxp commented Jun 10, 2024

closing due to inactivity

@fashxp fashxp closed this Jun 10, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jun 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants