Alex second contribution #21
Conversation
Starting with Neos 5, the font Noto Sans was missing in Neos.css. This has been fixed for Neos > 5 with neos/neos-development-collection#4005. So avoid using the Noto Sans font specifically for Neos 5.
Method return type declarations are supported since PHP 7.0 and this package is compatible with down to Neos 4.3, which requires PHP 7.1+.
|
The automatic redirection to the parent document has problems. Investigating .. |
.. and use documentNode instead of node consistently, as stated by Sebobo: "documentNode and node are sometimes the same thing, but documentNode makes it easier to read the code".
|
.. fixed. Has worked only for folders as child of regular pages, but not for nested folders. This is fixed and verified with @Sebobo :) |
|
I don't fully understand what is fixed here from the title & description. |
|
The Noto Sans is missing in Neos 5' Neos.css, and this package should be compatible down to Neos 4. So this package could either (1) dismiss the use of Noto Sans at all, or (2) it could always use |
|
On the one hand I think the package should raise its minimum Neos version at some point as Neos 4 and 5 are out of support anyways and not a lot of people would test changes here. This could be done with a minor version. And I don't get why the guest frame should try to load the font and get a 404. The Font definition already has a fallback and it can also be adjusted to try to use a local font instead. I mean the rendering of the Folder type is not that important IMO and doesn't have to be awesome looking. |
|
Besides I think it's great you try to fix this, but I think it's unnecessary complicated code in this package which should be lean and easy. |
|
For me it is a clean approach to add a switch for handling bugs in specific core versions. And this switch can be removed once the package is declared as no longer compatible with that core version. And for me, public packages are also a good place to present solutions to recurring problems that can be pointed out to other developers. But I can see that there is also an argument for keeping it as small as possible and not overdoing it. |
|
Hi Marvin, hi Sebobo, you might think that this PR is only about the missing font in Neos 5, but it's mainly about the correct redirection to the parent node in the frontend. I would be happy if this PR could be reviewed and approved. If you want to exclude the missing font handling from this PR, i will adapt the PR accordingly. Thanks and greetings, Alex |
|
@alexander-nitsche yes please break this apart. In the future please do one thing at a time and write a proper PR title to avoid confusion 👍 The status code should be 303 like Neos does internally for shortcuts. A permanent redirect would be a problem when pages or folders are being moved. |
|
Ok. Split off frontend redirect task to #22. |
|
Split off re-use of Noto Sans font to #23. |
|
This PR is redundant and can be closed. |
Hi Marvin,
i handled the missing Noto Sans font in Neos 5 (for later Neos versions this will be fixed with this core issue) and the automatic redirection to the folder's parent page in the frontend, which was also mentioned in issue #19 of this project.
I tested the patch in Neos 7 and made code comparisons with Neos 4 to ensure backward compatibility.
Greetings
Alex