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

fix: sorting of gridelement children within CType menu_section #12

Merged
merged 2 commits into from
Aug 17, 2021
Merged

fix: sorting of gridelement children within CType menu_section #12

merged 2 commits into from
Aug 17, 2021

Conversation

kitzberger
Copy link
Contributor

Due to their separate sorting order per grid column any grid child is placed on top of the menu rendered by CType menu_section. This fixes that situation by taking grid parent and column into the calculation of a custom order.

Inspired by http://blog.sitegefuehl.de/typo3-inhaltsverzeichnis-section-index-gridelements/

@schloram
Copy link
Member

schloram commented Aug 3, 2021

Hi @kitzberger ,
thanks for your contribution.

I'm not quite sure if this is something this extension should handle. I have to discuss this with a colleague first.

Furthermore this would be a feature (not a fix) since this isn't something which were broken in this extension.

regards,
Ramón

@kitzberger
Copy link
Contributor Author

@schloram, sure, check with your colleagues ;-)

Furthermore this would be a feature (not a fix) since this isn't something which were broken in this extension.

That's where I tend to disagree a little. It's something that our users couldn't live with so it needed a fix ;-) But I don't have any strong opinions about those kinda semantics, so whatever you like it to be it is ;-)

@schloram
Copy link
Member

schloram commented Aug 3, 2021

That's where I tend to disagree a little. It's something that our users couldn't live with so it needed a fix ;-) But I don't have any strong opinions about those kinda semantics, so whatever you like it to be it is ;-)

Sure. It's discussible. :) And to be honest not worth a headache :D

I'll get back to you when we have a decision.

@schloram
Copy link
Member

Hi @kitzberger ,
we discussed this with the following conclusion:

  • We think this is a good addition for this extension. 👍
  • We decided to treat this change as a feature. It is sure discussible and no approach is wrong, but we are more on the "feature" side (maybe 35/65 for "feature" 😄 ). But we can alter the commit message when we "Squash and merge". So nothing to do on your side.
  • Could you please include the "SiteGefühl" blog post link to the comment in the TypoScript? Like see: http://blog.sitegefuehl.de/typo3-inhaltsverzeichnis-section-index-gridelements/. So it's easier to determine why this code block is there and where you got your idea from. :)

But then I'll be happy to merge this. Good work.

@schloram schloram merged commit 18ef602 into itplusx:master Aug 17, 2021
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