Skip to content

SYL-4341 Remove documentation links #74

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

Merged
merged 12 commits into from
Mar 31, 2025

Conversation

Cholin2000
Copy link
Collaborator

@Cholin2000 Cholin2000 commented Mar 27, 2025

@Cholin2000 Cholin2000 marked this pull request as ready for review March 28, 2025 10:22
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if adding this macro makes sense, as these keys are used once 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just create form_theme for each field and set the links in there?
Then we don't have to if into oblivion, but have a block per case

@GSadee GSadee added Admin Admin Panel related issues and PRs. DX Issues and PRs aimed at improving Developer eXperience. labels Mar 28, 2025
@@ -1,12 +1,20 @@
{% import '@SyliusAdmin/PaymentMethod/_documentation_helpers.html.twig' as doc %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{% import '@SyliusAdmin/PaymentMethod/_documentation_helpers.html.twig' as doc %}
{% import '@SyliusMolliePlugin/Admin/PaymentMethod/_documentation_helpers.html.twig' as doc %}

Custom templates should live in the app

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

exaple
Maybe we should move it to a new PR, since there are many more like this here—it might be better to handle all of them at once. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just create form_theme for each field and set the links in there?
Then we don't have to if into oblivion, but have a block per case

@Cholin2000 Cholin2000 force-pushed the Remove-mollie-documentation-links branch from 7ca1764 to f2213e1 Compare March 28, 2025 13:50
Copy link
Member

Choose a reason for hiding this comment

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

Newly added templates should be added to the proper plugin templates directory, the others could be refactored in a separate PR. Could you open such a PR after merging this one? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure! 🫡

@GSadee GSadee merged commit ded81e3 into Sylius:2.0 Mar 31, 2025
3 checks passed
@GSadee GSadee deleted the Remove-mollie-documentation-links branch March 31, 2025 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin Admin Panel related issues and PRs. DX Issues and PRs aimed at improving Developer eXperience.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants