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

MINOR: better getter in example #61

Merged
merged 1 commit into from May 25, 2022
Merged

Conversation

sunnysideup
Copy link
Contributor

Awesome module!

Copy link
Owner

@bummzack bummzack left a comment

Choose a reason for hiding this comment

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

Thanks for your addition. How is the getManyManyComponents better than using the "magic" accessor?

I'd rather just keep one example to keep things terse.

@sunnysideup
Copy link
Contributor Author

sunnysideup commented May 25, 2022

I don't really understand what you mean. I understand the words, but I thought that my pull request made the "magic" accessor return the sorted list, and that the current example adds another method.

Just use as you see fit.

@bummzack
Copy link
Owner

Ah, I think I understand now. You're basically overriding the native accessor Images 🤔
Have you used that in production? I've always been hesitant to override the native accessors, as it will make it harder to access the unmodified data-source

@sunnysideup
Copy link
Contributor Author

We use this all the time in sortable gridfield: https://github.com/UndefinedOffset/SortableGridField/blob/master/docs/ManyManyExample.md... I totally agree and understand with your hesitation, but I can report that it seems to work most of the time.

@bummzack bummzack merged commit 6777f3c into bummzack:master May 25, 2022
@bummzack
Copy link
Owner

All good, thanks. I'll leave it up to the developer to decide

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.

None yet

2 participants