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

ITIL template field twig ui #16366

Merged
merged 3 commits into from
Jan 17, 2024
Merged

Conversation

cconard96
Copy link
Contributor

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -

Migrate form/list for hidden, readonly, mandatory and predefined ITIL template fields to Twig and de-duplicate the display code.

Copy link
Contributor

@orthagh orthagh left a comment

Choose a reason for hiding this comment

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

Same as previous pr, massive action button should be spaced.
Also in "mandatory fields" tab, there is a warning: "PHP Warning (2): Undefined array key 7 in /var/www/html/glpi/10.1.git/src/ITILTemplateField.php at line 170"

@cconard96
Copy link
Contributor Author

Same as previous pr, massive action button should be spaced. Also in "mandatory fields" tab, there is a warning: "PHP Warning (2): Undefined array key 7 in /var/www/html/glpi/10.1.git/src/ITILTemplateField.php at line 170"

I'm not sure that issue is unique to my changes in this PR. Do you have a field added from a plugin that isn't enabled anymore? Both with this PR and in the main branch, it seems to try accessing the name of all added fields from the list of allowed ones without a check to see if they still exist.

@orthagh
Copy link
Contributor

orthagh commented Jan 16, 2024

You're right, concerned option seems to be part of an inactive/uninstalled plugin

@trasher trasher merged commit 0409983 into glpi-project:main Jan 17, 2024
8 checks passed
}) }}
{{ extra_form_html|raw }}
{% if show_submit %}
<div class="ms-2">{{ inputs.submit('add', _x('button', 'Add')) }}</div>
Copy link
Member

Choose a reason for hiding this comment

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

@cconard96 _x() will not be detected by xgettext here.

I suggest to move this template into a dedicated Twig file. You could, inside this file, include components/datatable.html.twig, in order to have a single call to TemplateRenderer::getInstance()->display() inside the current method.

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