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.

Sign up for GitHub

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

Jump to bottom

Initial view helpers #1

Merged
merged 13 commits into from
Nov 26, 2024
Merged

Initial view helpers #1

merged 13 commits into from
Nov 26, 2024

Conversation

gsteel
Copy link
Member

@gsteel gsteel commented Aug 8, 2022 •
edited
Loading

Adds ServerUrl and Url view helpers along with a a factory for the Doctype view helper.

This first PR is just getting things off the ground without there being too much to review. In future PRs, I'll work on relocating the ViewEvent, Rendering strategies, more relevant factories and configuration etc.

This was referenced Aug 8, 2022
Closed
Closed
@gsteel gsteel added the enhancement New feature or request label Nov 9, 2022
@gsteel gsteel added this to the 1.0.0 milestone Nov 9, 2022
@gsteel gsteel force-pushed the initial-view-helpers branch 2 times, most recently from e056dbf to 910e57c Compare November 9, 2022 08:43
gsteel added 9 commits November 9, 2022 08:44
@gsteel
Adds replacement ServerUrl helper
7f40bc0 
Signed-off-by: George Steel <[email protected]>
@gsteel
config.platform should be 7.4.99
b5c7444 
Signed-off-by: George Steel <[email protected]>
@gsteel
Introduce a thin wrapper around webmozart/assert marking as @internal
e160df6 
Signed-off-by: George Steel <[email protected]>
@gsteel
Introduce a factory for the Doctype view helper.
2dac760 
This factory and default configuration preserves the historic method of configuring the doctype for an MVC application

Signed-off-by: George Steel <[email protected]>
@gsteel
Add Url helper to preserve BC with legacy Laminas\View\Helper\Url
f29846f 
Signed-off-by: George Steel <[email protected]>
@gsteel
Removes affordance to the deprecated ModuleRouteListener in Url helper
70fcbc6 
Ref: laminas/laminas-mvc#123
Signed-off-by: George Steel <[email protected]>
@gsteel
Adds notes for changes required to MVC
ffbcaf6 
Signed-off-by: George Steel <[email protected]>
@gsteel
Updates locked dependencies
7dc6604 
Signed-off-by: George Steel <[email protected]>
@gsteel
Drop support for PHP 7.4, add support for PHP 8.2
23f947e 
Signed-off-by: George Steel <[email protected]>
@gsteel gsteel force-pushed the initial-view-helpers branch from 910e57c to 23f947e Compare November 9, 2022 08:45
boesing
boesing reviewed Nov 9, 2022
View reviewed changes
src/Assert.php Show resolved Hide resolved
@boesing
Copy link
Member

boesing commented Nov 9, 2022

I do not use laminas-view anywhere and thus I am not the correct person to properly review this from a user PoV.
Could you probably give a quick glance on how this is actually "different" to the existing "implementation"?

I assume that most of the code is just transferred to a new namespace?

boesing
boesing requested changes Nov 9, 2022
View reviewed changes
Copy link
Member

@boesing boesing left a comment

Choose a reason for hiding this comment

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

Just a lil change request here regarding the psalm-baseline.

psalm-baseline.xml Show resolved Hide resolved
psalm.xml Show resolved Hide resolved
src/Assert.php Show resolved Hide resolved
@gsteel
Copy link
Member Author

gsteel commented Nov 9, 2022

I do not use laminas-view anywhere and thus I am not the correct person to properly review this from a user PoV. Could you probably give a quick glance on how this is actually "different" to the existing "implementation"?

I assume that most of the code is just transferred to a new namespace?

Ignoring the factories and config, this PR only introduces the Url and ServerUrl view helpers. These helpers are specific to mvc and largely unchanged from what is currently in view.

Once released, view will be able to drop a bunch of deps, like laminas-http

weierophinney
weierophinney requested changes Nov 21, 2022
View reviewed changes
src/Assert.php Show resolved Hide resolved
src/ConfigProvider.php Outdated Show resolved Hide resolved
src/Helper/Url.php Show resolved Hide resolved
src/Helper/Url.php Show resolved Hide resolved
src/Module.php Outdated Show resolved Hide resolved
gsteel added 2 commits July 18, 2023 23:12
@gsteel
Bump all dependencies
73539b6 
- Upgrade psalm to v5, handle deprecations and other new issues such as unused code
- Upgrade PHPUnit to 10.x, migrate config and tests
- Drop support for PHP 8.0
- Add psalm type for view helper config array shape

Signed-off-by: George Steel <[email protected]>
@gsteel
Remove empty method for dependency configuration
e01b72e 
Signed-off-by: George Steel <[email protected]>
@gsteel gsteel requested a review from weierophinney July 18, 2023 22:17
froschdesign
froschdesign reviewed Jul 19, 2023
View reviewed changes
MVC-TODO.md Outdated Show resolved Hide resolved
@gsteel
Remove TODO list
016f5e3 
Signed-off-by: George Steel <[email protected]>
froschdesign
froschdesign approved these changes Nov 26, 2024
View reviewed changes
Copy link
Member

@froschdesign froschdesign left a comment

Choose a reason for hiding this comment

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

A very good first step! Go for it. 👍🏻

@gsteel gsteel self-assigned this Nov 26, 2024
@gsteel gsteel merged commit 73aa8dc into 1.0.x Nov 26, 2024
@gsteel gsteel deleted the initial-view-helpers branch November 26, 2024 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@internalsystemerror internalsystemerror internalsystemerror left review comments

@boesing boesing boesing requested changes

@froschdesign froschdesign froschdesign approved these changes

@weierophinney weierophinney Awaiting requested review from weierophinney

Assignees

@gsteel gsteel

Labels
enhancement New feature or request
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@gsteel @boesing @weierophinney @froschdesign @internalsystemerror