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

Allow record sliding in page.resources.fal with traits #825

Merged
merged 4 commits into from
Aug 8, 2015
Merged

Allow record sliding in page.resources.fal with traits #825

merged 4 commits into from
Aug 8, 2015

Conversation

yol
Copy link
Contributor

@yol yol commented Apr 27, 2015

Continuation from #679 - now with 100% more traits.

}

/**
* @return \TYPO3\CMS\Extbase\Object\ObjectManagerInterface
Copy link
Member

Choose a reason for hiding this comment

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

@return FluidTYPO3\Vhs\Service\PageSelectService

@NamelessCoder
Copy link
Member

Damn this looks good. Strike that - damn, this looks really good! I think there's only nit-picking for style and meta-doc and then adjusting the unit tests (and for 1,000 bonus points, a basic test or two to cover the behavior of this trait!).

Record sliding was traditionally handled solely by content view helpers. It is however desirable to extend sliding support to a greater range of view helpers, most prominently Page\Resources\FalViewHelper.

SlideViewHelperTrait implements all functionality commonly required for sliding.

Compared to the original implementation in Content\RenderViewHelper, a few changes were introduced:
* $limit is now referring to the total number of records, not the number of records rendered per page. This is important when using $slideCollect.
* $slideCollect in combination with $slideCollectReverse now selects the correct page range from the rootline when setting it to a value greater than zero.
* $slideCollectReverse has no effect when $slideCollect is zero.
This is achieved by utilizing the new functionality introduced by SlideViewHelperTrait. Custom ordering of the records is not supported, but probably also not needed.
The sliding-related code from AbstractContentViewHelper was adopted into SlideViewHelperTrait and thus does not need to be duplicated here.
…cord sliding

Content sliding was already supported, but lacked some features. Now it is possible to use $slideCollect and render more than one randomly selected content element by increasing $limit. A change in the inheritance hierarchy was necessary, but should not have any impact on the behaviour.
@yol
Copy link
Contributor Author

yol commented Jun 16, 2015

I've taken care of the meta-doc miss you pointed out. As far as I can see, none of the existing unit tests need to be changed since they don't do much anyway.
I've also looked into making unit tests for the slide behaviour, but I have zero experience with PHPUnit/TYPO3 unit testing and am completely lost on how to intercept the TYPO3_DB SQL queries and provide static fixtures as results.

@xf-
Copy link
Contributor

xf- commented Jul 26, 2015

Hi,
we need to discuss this. This would mean we would drop 6.2 requirements, because traits are introduced in PHP 5.4 and TYPO3 6.2 supports 5.3.7+ and Debian squeeze LTS is until 2016-02.

Personally i would love to drop 5.3 and also use standard array syntax with [] and no extending with [] and definition array().

@yol
Copy link
Contributor Author

yol commented Jul 26, 2015

I don't think there's any change from the status quo. vhs already requires PHP 5.4 since traits were introduced in other ViewHelpers about 5 months ago by NamelessCoder.

@xf-
Copy link
Contributor

xf- commented Jul 26, 2015

@pkerling ok, that means we drop 5.3 :)

@yol
Copy link
Contributor Author

yol commented Jul 26, 2015

Just had a look at ext_emconf.php, 5.3 is gone already since TER version 2.3.0 (see commit d1b732d)

@xf-
Copy link
Contributor

xf- commented Jul 26, 2015

OK thx :)

@NamelessCoder NamelessCoder added this to the VHS 2.3.4 milestone Aug 8, 2015
NamelessCoder added a commit that referenced this pull request Aug 8, 2015
Allow record sliding in page.resources.fal with traits
@NamelessCoder NamelessCoder merged commit 4b2215d into FluidTYPO3:development Aug 8, 2015
@NamelessCoder
Copy link
Member

Thank you so much again, @pkerling - not least for your patience with this one :)

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.

4 participants