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

NEW Show nested DataObjects on a single row in the file used on table #1097

Conversation

emteknetnz
Copy link
Member

@emteknetnz emteknetnz commented Aug 25, 2020

Issue #1098

Investigation issue https://github.com/silverstripeltd/product-issues/issues/249

DON'T MERGE YET

build with yarn dev for now because jest tests haven't yet been updated

image

];
$this->extend('updateUsageArray', $arr, $dataObject);
Copy link
Member Author

Choose a reason for hiding this comment

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

This whole approach of looking ->Parent() may be a little pointless in that there's not many (any?) non-page DataObjects that follow this pattern (at least in kitchen-sink). It may be more sense to not look for ->Parent() by default and instead rely on this new extension hook.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about File and Folder or TaxonomyTerm? Those come to mind as using the Hierarchy extension which provides has_one Parent.

Copy link
Contributor

@sachajudd sachajudd left a comment

Choose a reason for hiding this comment

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

Could you please remove the two font-weight's of 500 from your PR? These should just be default which is 400 and will come through automatically. I'll update the designs to showcase this more clearly. Could you please ensure the title is 13px and the description is 11px. I didn't pull down your branch to check but please request another review from me when you're ready for a UX review.

@emteknetnz
Copy link
Member Author

@sachajudd sorry I should have updated description. This PR won't get merged as is basically just used as reference for future work, possibly some of it will be copy-pasted. We ended up spinning up a new issue for used on table feature work.
#1098

I'll close this PR.

@emteknetnz emteknetnz closed this Sep 9, 2020
@emteknetnz
Copy link
Member Author

emteknetnz commented Sep 25, 2020

Code from this PR has been somewhat moved to #1121

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

3 participants