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

Feature: allow attributes for <fieldset> wrapper #253

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
27b463b
reduce complexity
mimmi20 Jan 9, 2024
83c1e16
revert change
mimmi20 Jan 9, 2024
96e98c1
revert changes
mimmi20 Jan 9, 2024
a6c287d
revert issue
mimmi20 Jan 9, 2024
1c9ac51
Merge pull request #259 from mimmi20/feature-reduce-complexity
Xerkus Jan 9, 2024
2cf5a72
Improve type inference for FormElementManager::get()
gsteel Jan 9, 2024
f12cd91
Form element manager always returns an element interface or throws an…
gsteel Jan 10, 2024
cb68f54
Baseline issue with `FormElementManager::get()` related to inheritance
gsteel Jan 10, 2024
dcd82ee
Prefer `@return` over `@psalm-return`
gsteel Jan 10, 2024
60f1cea
Assert that a class string not implementing ElementInterface is treat…
gsteel Jan 10, 2024
ee4a3b6
Merge pull request #260 from gsteel/form-element-manager-template
Xerkus Jan 10, 2024
ffdf00a
Update docs-build workflow
Xerkus Jan 10, 2024
332a31a
Add failing test case for integer element labels
gsteel Jan 11, 2024
cdeb149
Expand types for `translateLabel()` argument to preserve legacy BC.
gsteel Jan 11, 2024
c6f6c68
Merge pull request #261 from gsteel/Integer-Label-BC-Break
Xerkus Jan 11, 2024
547c144
Merge pull request #262 from laminas/3.19.x-merge-up-into-3.20.x_MkCX…
gsteel Jan 11, 2024
0a2e50f
allow attributes for <fieldset> wrapper
mimmi20 Jan 3, 2024
4047965
remove import
mimmi20 Jan 3, 2024
caeb424
update helper
mimmi20 Jan 4, 2024
c20eed0
updates
mimmi20 Jan 4, 2024
115449e
allow attributes for <fieldset> wrapper
mimmi20 Jan 3, 2024
df79f8e
remove import
mimmi20 Jan 3, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions .github/workflows/docs-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ name: docs-build
on:
release:
types: [published]
repository_dispatch:
types: docs-build
workflow_dispatch:

jobs:
build-deploy:
Expand All @@ -13,5 +12,5 @@ jobs:
- name: Build Docs
uses: laminas/documentation-theme/github-actions/docs@master
env:
"DOCS_DEPLOY_KEY": ${{ secrets.DOCS_DEPLOY_KEY }}
"GITHUB_TOKEN": ${{ secrets.GITHUB_TOKEN }}
DEPLOY_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
6 changes: 6 additions & 0 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,9 @@
<code>Element\DateTime::class</code>
<code>Element\DateTime::class</code>
</DeprecatedClass>
<LessSpecificReturnStatement>
<code>parent::get($name, $options)</code>
</LessSpecificReturnStatement>
<NonInvariantDocblockPropertyType>
<code>$aliases</code>
<code>$factories</code>
Expand Down Expand Up @@ -292,6 +295,9 @@
<code>translate</code>
<code>translate</code>
</PossiblyNullReference>
<RedundantCastGivenDocblockType>
<code>(string) $label</code>
</RedundantCastGivenDocblockType>
</file>
<file src="src/View/Helper/Form.php">
<DeprecatedMethod>
Expand Down
5 changes: 3 additions & 2 deletions src/FormElementManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -321,9 +321,10 @@ public function configure(array $config)
* createFromInvokable() will use these and pass them to the instance
* constructor if not null and a non-empty array.
*
* @param class-string<ElementInterface>|string $name Service name of plugin to retrieve.
* @template T of ElementInterface
* @param class-string<T>|string $name Service name of plugin to retrieve.
* @param null|array<mixed> $options Options to use when creating the instance.
* @psalm-return ($name is class-string<ElementInterface> ? ElementInterface : mixed)
* @return ($name is class-string<ElementInterface> ? T : ElementInterface)
*/
public function get($name, ?array $options = null): mixed
{
Expand Down
4 changes: 2 additions & 2 deletions src/View/Helper/AbstractFormDateSelect.php
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,13 @@ protected function getMonthsOptions(string $pattern): array
* NOTE: we don't use a pattern for years, as years written as two digits can lead to hard to
* read date for users, so we only use four digits years
*
* @return array
* @return array<int, string>
*/
protected function getYearsOptions(int $minYear, int $maxYear): array
{
$result = [];
for ($i = $maxYear; $i >= $minYear; --$i) {
$result[$i] = $i;
$result[$i] = (string) $i;
}

return $result;
Expand Down
9 changes: 7 additions & 2 deletions src/View/Helper/AbstractHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -561,12 +561,17 @@ protected function hasAllowedPrefix(string $attribute): bool
}

/**
* translate the label
* Translate the label
*
* @internal
*
* @todo Reduce argument to only string in the next major
* @param string $label
*/
protected function translateLabel(string|int $label): string|int
protected function translateLabel(int|string|float|bool $label): string
{
$label = (string) $label;

return $this->getTranslator()?->translate($label, $this->getTranslatorTextDomain()) ?? $label;
}

Expand Down
5 changes: 3 additions & 2 deletions src/View/Helper/FormCheckbox.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ public function render(ElementInterface $element): string
{
if (! $element instanceof CheckboxElement) {
throw new Exception\InvalidArgumentException(sprintf(
'%s requires that the element is of type Laminas\Form\Element\Checkbox',
__METHOD__
'%s requires that the element is of type %s',
__METHOD__,
CheckboxElement::class
));
}

Expand Down
133 changes: 102 additions & 31 deletions src/View/Helper/FormCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@

namespace Laminas\Form\View\Helper;

use Laminas\Form\Fieldset as FieldsetElement;
use Laminas\Form\Element\Collection as CollectionElement;

Check failure on line 8 in src/View/Helper/FormCollection.php

View workflow job for this annotation

GitHub Actions / ci / QA Checks (PHPCodeSniffer [8.1, locked], ubuntu-latest, laminas/laminas-continuous-integration-ac...

Use statements should be sorted alphabetically. The first wrong one is Laminas\Form\Element\Collection.
use Laminas\Form\ElementInterface;
use Laminas\Form\Exception;
use Laminas\Form\FieldsetInterface;
use Laminas\Form\LabelAwareInterface;
use Laminas\View\Helper\HelperInterface;
use RuntimeException;

Expand Down Expand Up @@ -47,7 +50,7 @@
*
* @var string
*/
protected $labelWrapper = '<legend>%s</legend>';
protected $labelWrapper = '<legend%1$s>%2$s</legend>';

/**
* Where shall the template-data be inserted into
Expand Down Expand Up @@ -77,6 +80,13 @@
*/
protected $fieldsetHelper;

/**
* Form label helper instance
*
* @var null|FormLabel
*/
protected $labelHelper;

/**
* Invoke helper as function
*
Expand All @@ -103,6 +113,14 @@
*/
public function render(ElementInterface $element): string
{
if (! $element instanceof FieldsetElement) {
throw new Exception\InvalidArgumentException(sprintf(
'%s requires that the element is of type %s',
__METHOD__,
FieldsetElement::class
));
}

$renderer = $this->getView();
if ($renderer !== null && ! method_exists($renderer, 'plugin')) {
// Bail early if renderer is not pluggable
Expand All @@ -113,6 +131,7 @@
$templateMarkup = '';
$elementHelper = $this->getElementHelper();
assert(is_callable($elementHelper));

$fieldsetHelper = $this->getFieldsetHelper();
assert(is_callable($fieldsetHelper));

Expand All @@ -128,43 +147,33 @@
}
}

if (! $this->shouldWrap) {
return $markup . $templateMarkup;
}

// Every collection is wrapped by a fieldset if needed
if ($this->shouldWrap) {
$attributes = $element->getAttributes();
if (! $this->getDoctypeHelper()->isHtml5()) {
unset(
$attributes['name'],
$attributes['disabled'],
$attributes['form']
);
}
$attributesString = $attributes !== [] ? ' ' . $this->createAttributesString($attributes) : '';
$attributes = $element->getAttributes();
if (! $this->getDoctypeHelper()->isHtml5()) {
unset(
$attributes['name'],
$attributes['disabled'],
$attributes['form']
);
}

$label = $element->getLabel();
$legend = '';
$label = $element->getLabel() ?? '';
$labelAttributes = [];

if (! empty($label)) {
$label = $this->translateLabel($label);
$label = $this->escapeLabel($element, $label);
if ($label !== '') {
$label = $this->translateLabel($label);
$label = $this->escapeLabel($element, $label);

$legend = sprintf(
$this->labelWrapper,
$label
);
if ($element instanceof LabelAwareInterface) {
$labelAttributes = $element->getLabelAttributes();
}

$markup = sprintf(
$this->wrapper,
$markup,
$legend,
$templateMarkup,
$attributesString
);
} else {
$markup .= $templateMarkup;
}

return $markup;
return $this->wrapElement($markup, $templateMarkup, $label, $attributes, $labelAttributes);
}

/**
Expand Down Expand Up @@ -371,4 +380,66 @@

return $this;
}

/**
* Retrieve the FormLabel helper
*/
protected function getLabelHelper(): FormLabel
{
if ($this->labelHelper) {
return $this->labelHelper;
}

if ($this->view !== null && method_exists($this->view, 'plugin')) {
$this->labelHelper = $this->view->plugin('form_label');
}

if (! $this->labelHelper instanceof FormLabel) {
$this->labelHelper = new FormLabel();
}

if ($this->hasTranslator()) {
$this->labelHelper->setTranslator(
$this->getTranslator(),
$this->getTranslatorTextDomain()
);
}

return $this->labelHelper;
}

public function wrapLabel(string $label, array $labelAttributes = []): string
{
$labelHelper = $this->getLabelHelper();
$labelAttributesString = '';

if (is_array($labelAttributes) && $labelAttributes !== []) {

Check failure on line 416 in src/View/Helper/FormCollection.php

View workflow job for this annotation

GitHub Actions / ci / QA Checks (PHPCodeSniffer [8.1, locked], ubuntu-latest, laminas/laminas-continuous-integration-ac...

Function is_array() should not be referenced via a fallback global name, but via a use statement.

Check failure on line 416 in src/View/Helper/FormCollection.php

View workflow job for this annotation

GitHub Actions / ci / QA Checks (Psalm [8.1, locked], ubuntu-latest, laminas/laminas-continuous-integration-action@v1, ...

RedundantCondition

src/View/Helper/FormCollection.php:416:13: RedundantCondition: Type array<array-key, mixed> for $labelAttributes is always array<array-key, mixed> (see https://psalm.dev/122)
$labelAttributesString = ' ' . $labelHelper->createAttributesString($labelAttributes);
}

return sprintf(
$this->getLabelWrapper(),
$labelAttributesString,
$label
);
}

public function wrapElement(string $markup, string $templateMarkup, string $label, array $attributes = [], array $labelAttributes = []): string

Check warning on line 427 in src/View/Helper/FormCollection.php

View workflow job for this annotation

GitHub Actions / ci / QA Checks (PHPCodeSniffer [8.1, locked], ubuntu-latest, laminas/laminas-continuous-integration-ac...

Line exceeds 120 characters; contains 147 characters
{
$legend = '';

if ($label !== '') {
$legend = $this->wrapLabel($label, $labelAttributes);
}

$attributesString = $attributes !== [] ? ' ' . $this->createAttributesString($attributes) : '';

return sprintf(
$this->getWrapper(),
$markup,
$legend,
$templateMarkup,
$attributesString
);
}
}
5 changes: 3 additions & 2 deletions src/View/Helper/FormDateSelect.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ public function render(ElementInterface $element): string
{
if (! $element instanceof DateSelectElement) {
throw new Exception\InvalidArgumentException(sprintf(
'%s requires that the element is of type Laminas\Form\Element\DateSelect',
__METHOD__
'%s requires that the element is of type %s',
__METHOD__,
DateSelectElement::class
));
}

Expand Down
5 changes: 3 additions & 2 deletions src/View/Helper/FormDateTimeSelect.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,9 @@ public function render(ElementInterface $element): string
{
if (! $element instanceof DateTimeSelectElement) {
throw new Exception\InvalidArgumentException(sprintf(
'%s requires that the element is of type Laminas\Form\Element\DateTimeSelect',
__METHOD__
'%s requires that the element is of type %s',
__METHOD__,
DateTimeSelectElement::class
));
}

Expand Down
5 changes: 3 additions & 2 deletions src/View/Helper/FormMonthSelect.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ public function render(ElementInterface $element): string
{
if (! $element instanceof MonthSelectElement) {
throw new Exception\InvalidArgumentException(sprintf(
'%s requires that the element is of type Laminas\Form\Element\MonthSelect',
__METHOD__
'%s requires that the element is of type %s',
__METHOD__,
MonthSelectElement::class
));
}

Expand Down
5 changes: 3 additions & 2 deletions src/View/Helper/FormMultiCheckbox.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,9 @@ public function render(ElementInterface $element): string
{
if (! $element instanceof MultiCheckboxElement) {
throw new Exception\InvalidArgumentException(sprintf(
'%s requires that the element is of type Laminas\Form\Element\MultiCheckbox',
__METHOD__
'%s requires that the element is of type %s',
__METHOD__,
MultiCheckboxElement::class
));
}

Expand Down
Loading