issue_155 #156

Closed
maieul wants to merge 6 commits from issue_155 into master
maieul commented 11 months ago
Owner

Ceci résoud #155 :

  • saisies_deplacer_avant()
  • saisies_deplacer_apres()
  • Si le paramètre $ou est de la forme [conteneur][xx]xx est un entier (à partir de zero), on insère à la position xx dans le conteneur.
Ceci résoud #155 : - `saisies_deplacer_avant()` - `saisies_deplacer_apres()` + Si le paramètre `$ou` est de la forme `[conteneur][xx]` où `xx` est un entier (à partir de zero), on insère à la position `xx` dans le conteneur.
maieul added 4 commits 11 months ago
Owner

super, merci maieul, je teste dès que possible

super, merci maieul, je teste dès que possible
Poster
Owner

des retours ? j'aimerai releaser prochainement

des retours ? j'aimerai releaser prochainement

j'ai pas de quoi tester sous la main mais le code à l'air correc'

j'ai pas de quoi tester sous la main mais le code à l'air correc'
Poster
Owner

On va dire que c'est le cas, vue ta lecture + le fait que j'ai mis aussi des regressions tests.

On va dire que c'est le cas, vue ta lecture + le fait que j'ai mis aussi des regressions tests.
Owner

Je me prévois un test en fin d'aprèm (c'est surtout quand $ou est de la forme [conteneur][xx] que j'ai jamais essayé).

Je me prévois un test en fin d'aprèm (c'est surtout quand `$ou` est de la forme `[conteneur][xx]` que j'ai jamais essayé).
Poster
Owner

@tcharlss oki, alors j'attend.

@tcharlss oki, alors j'attend.
Owner

Je suis parti d'un formulaire ayant cette structure :

|_ racine 1
|_ fieldset
   |_ enfant 1
   |_ enfant 2
|_ racine 2
|_ racine 3
|_ saisie à déplacer

...et testé en déplaçant la dernière avant et après toutes les autres.

  • saisies_deplacer_apres() : ok, fonctionne dans tous les cas.
  • saisies_deplacer_avant() : problème, ça positionne après la cible.

Le formulaire de test :

function formulaires_test_deplacer_saisies_dist() {

	$saisies = [
		[
			'saisie' => 'input',
			'options' => [
				'nom' => 'racine1',
				'label' => 'Racine 1'
			]
		],
		[
			'saisie' => 'fieldset',
			'options' => [
				'nom' => 'parent',
			],
			'saisies' => [
				[
					'saisie' => 'input',
					'options' => [
						'nom' => 'enfant1',
						'label' => 'Enfant 1',
					]
				],
				[
					'saisie' => 'input',
					'options' => [
						'nom' => 'enfant2',
						'label' => 'Enfant 2',
					]
				],
			],
		],
		[
			'saisie' => 'input',
			'options' => [
				'nom' => 'racine2',
				'label' => 'Racine 2'
			]
		],
		[
			'saisie' => 'input',
			'options' => [
				'nom' => 'cible',
				'label' => 'Saisie déplacée',
				'conteneur_class' => 'highlight',
			]
		],
	];

	include_spip('inc/saisies');
	// OK
	// $saisies = saisies_deplacer_apres($saisies, 'cible', 'racine1');
	// $saisies = saisies_deplacer_apres($saisies, 'cible', 'enfant1');
	// $saisies = saisies_deplacer_apres($saisies, 'cible', 'enfant2');
	// $saisies = saisies_deplacer_apres($saisies, 'cible', 'parent');
	// $saisies = saisies_deplacer_apres($saisies, 'cible', 'racine2');
	// $saisies = saisies_deplacer_apres($saisies, 'cible', '[parent][0]');
	// $saisies = saisies_deplacer_apres($saisies, 'cible', '[parent][1]');

	// PAS OK : positionne après la cible
	// $saisies = saisies_deplacer_avant($saisies, 'cible', 'racine1');
	// $saisies = saisies_deplacer_avant($saisies, 'cible', 'enfant1');
	// $saisies = saisies_deplacer_avant($saisies, 'cible', 'enfant2');
	// $saisies = saisies_deplacer_avant($saisies, 'cible', 'parent');
	// $saisies = saisies_deplacer_avant($saisies, 'cible', 'racine2');
	// $saisies = saisies_deplacer_avant($saisies, 'cible', '[parent][0]'); // 2ème position
	// $saisies = saisies_deplacer_avant($saisies, 'cible', '[parent][1]'); // 3ème position

	return $saisies;
}

Ah au passage il y a un warning quand on indique une cible qui n'existe pas (php 7.4) :

Warning: count(): Parameter must be an array or an object that implements Countable in inc/saisies_manipuler.php on line 131
Je suis parti d'un formulaire ayant cette structure : ``` |_ racine 1 |_ fieldset |_ enfant 1 |_ enfant 2 |_ racine 2 |_ racine 3 |_ saisie à déplacer ``` ...et testé en déplaçant la dernière avant et après toutes les autres. * `saisies_deplacer_apres()` : ok, fonctionne dans tous les cas. * `saisies_deplacer_avant()` : problème, ça positionne _après_ la cible. Le formulaire de test : ```php function formulaires_test_deplacer_saisies_dist() { $saisies = [ [ 'saisie' => 'input', 'options' => [ 'nom' => 'racine1', 'label' => 'Racine 1' ] ], [ 'saisie' => 'fieldset', 'options' => [ 'nom' => 'parent', ], 'saisies' => [ [ 'saisie' => 'input', 'options' => [ 'nom' => 'enfant1', 'label' => 'Enfant 1', ] ], [ 'saisie' => 'input', 'options' => [ 'nom' => 'enfant2', 'label' => 'Enfant 2', ] ], ], ], [ 'saisie' => 'input', 'options' => [ 'nom' => 'racine2', 'label' => 'Racine 2' ] ], [ 'saisie' => 'input', 'options' => [ 'nom' => 'cible', 'label' => 'Saisie déplacée', 'conteneur_class' => 'highlight', ] ], ]; include_spip('inc/saisies'); // OK // $saisies = saisies_deplacer_apres($saisies, 'cible', 'racine1'); // $saisies = saisies_deplacer_apres($saisies, 'cible', 'enfant1'); // $saisies = saisies_deplacer_apres($saisies, 'cible', 'enfant2'); // $saisies = saisies_deplacer_apres($saisies, 'cible', 'parent'); // $saisies = saisies_deplacer_apres($saisies, 'cible', 'racine2'); // $saisies = saisies_deplacer_apres($saisies, 'cible', '[parent][0]'); // $saisies = saisies_deplacer_apres($saisies, 'cible', '[parent][1]'); // PAS OK : positionne après la cible // $saisies = saisies_deplacer_avant($saisies, 'cible', 'racine1'); // $saisies = saisies_deplacer_avant($saisies, 'cible', 'enfant1'); // $saisies = saisies_deplacer_avant($saisies, 'cible', 'enfant2'); // $saisies = saisies_deplacer_avant($saisies, 'cible', 'parent'); // $saisies = saisies_deplacer_avant($saisies, 'cible', 'racine2'); // $saisies = saisies_deplacer_avant($saisies, 'cible', '[parent][0]'); // 2ème position // $saisies = saisies_deplacer_avant($saisies, 'cible', '[parent][1]'); // 3ème position return $saisies; } ``` Ah au passage il y a un warning quand on indique une cible qui n'existe pas (php 7.4) : ``` Warning: count(): Parameter must be an array or an object that implements Countable in inc/saisies_manipuler.php on line 131 ```
Owner

En parlant de saisies_inserer et saisies_deplacer, une remarque à propos du paramètre $ou, cf. docbloc :

@param string       $ou
 *
 *	- Le nom de la saisie devant laquelle on déplacera
 *	- OU le nom d'un conteneur entre crochets [conteneur] (et dans ce cas on déplace à la fin de conteneur)
 *	- OU le nom d'un conteneur entre crochets suivi d'un identifiant numérique entre crochets [conteneur][x] (et dans ce cas on déplace à la position x au sein du conteneur)

Pour les crochets je vois l'idée, mais je trouve que cette syntaxe porte un peu à confusion : le nom des saisies peut déjà contenir des crochets de base. Alors certes pour des fieldsets ça n'a pas trop d'intérêt, mais ça n'est pas interdit.
Et ça fait une pseudo-syntaxe de plus à mémoriser.

Je pense que ça serait plus clair et plus simple si ce paramètre prenait tout le temps le nom de la saisie ciblée et rien d'autre, complété d'un paramètre supplémentaire $position pour indiquer la position souhaitée si la cible est un conteneur.

D'un coup d'oeil je comprends mieux ceci :

saisies_inserer($saisies, $nouvelle_saisie, 'le_fieldset', 2)

que cela, qui me fait aller relire la doc pour comprendre :

saisies_inserer($saisies, $nouvelle_saisie, '[le_fieldset][2]')

Bon désolé un peu hs, on pourra en discuter plus tard dans un autre ticket, je note juste pour pas oublier.

En parlant de saisies_inserer et saisies_deplacer, une remarque à propos du paramètre `$ou`, cf. docbloc : ```php @param string $ou * * - Le nom de la saisie devant laquelle on déplacera * - OU le nom d'un conteneur entre crochets [conteneur] (et dans ce cas on déplace à la fin de conteneur) * - OU le nom d'un conteneur entre crochets suivi d'un identifiant numérique entre crochets [conteneur][x] (et dans ce cas on déplace à la position x au sein du conteneur) ``` Pour les crochets je vois l'idée, mais je trouve que cette syntaxe porte un peu à confusion : le nom des saisies peut déjà contenir des crochets de base. Alors certes pour des fieldsets ça n'a pas trop d'intérêt, mais ça n'est pas interdit. Et ça fait une pseudo-syntaxe de plus à mémoriser. Je pense que ça serait plus clair et plus simple si ce paramètre prenait tout le temps le nom de la saisie ciblée et rien d'autre, complété d'un paramètre supplémentaire `$position` pour indiquer la position souhaitée si la cible est un conteneur. D'un coup d'oeil je comprends mieux ceci : ```php saisies_inserer($saisies, $nouvelle_saisie, 'le_fieldset', 2) ``` que cela, qui me fait aller relire la doc pour comprendre : ```php saisies_inserer($saisies, $nouvelle_saisie, '[le_fieldset][2]') ``` Bon désolé un peu hs, on pourra en discuter plus tard dans un autre ticket, je note juste pour pas oublier.
Poster
Owner

oui, autre ticket. C'est une très vieille syntaxe historique, d'avant même que jintervienne :)

oui, autre ticket. C'est une très vieille syntaxe historique, d'avant même que jintervienne :)
Poster
Owner

Ah et une remarque : non les crochets ont du sens.

saisie_inserer($saisies, $saisie, 'conteneur')

insère $saisie avant la saisie conteneur

alors que

saisie_inserer($saisies, $saisie, '[conteneur]') insère au début du conteneur.

Ah et une remarque : non les crochets ont du sens. `saisie_inserer($saisies, $saisie, 'conteneur')` insère `$saisie` avant la saisie `conteneur` alors que `saisie_inserer($saisies, $saisie, '[conteneur]')` insère au début du `conteneur`.
Poster
Owner

Ah au passage il y a un warning quand on indique une cible qui n'existe pas (php 7.4) :

je ne reproduis pas, mais je ne suis pas su de comprendre ce que tu as tenté de faire :)

> Ah au passage il y a un warning quand on indique une cible qui n'existe pas (php 7.4) : je ne reproduis pas, mais je ne suis pas su de comprendre ce que tu as tenté de faire :)
maieul force-pushed issue_155 from d836adb39d to 3b87ad8c3a 10 months ago
Poster
Owner

@tcharlss peut tu retester ? une fausse manip lors de la création de saisies_deplacer_apres m'avait fait modifié le code de saisies_deplacer_avant + 1 petite bricole... et j'avais oublié de reverifier les tests unitaires.

Mais voilà qui est corrigé. J'ai aussi pompé tes tests pour les intégrer dans les tests unitaires... mais il est tard, et donc une dernière verif à la main m'irait bien.

@tcharlss peut tu retester ? une fausse manip lors de la création de saisies_deplacer_apres m'avait fait modifié le code de saisies_deplacer_avant + 1 petite bricole... et j'avais oublié de reverifier les tests unitaires. Mais voilà qui est corrigé. J'ai aussi pompé tes tests pour les intégrer dans les tests unitaires... mais il est tard, et donc une dernière verif à la main m'irait bien.
Owner

Tout à l'air bon cette fois-ci, merci

je ne reproduis pas, mais je ne suis pas su de comprendre ce que tu as tenté de faire :)

Bizarrement ça ne se produit qu'avec saisies_inserer_apres :

$saisies = saisies_inserer_apres($saisies, $nouvelle_saisie, 'saisie_qui_n_existe_pas');

edi : mais bon c'est pas lié à cette PR, je note au passage.

Tout à l'air bon cette fois-ci, merci > je ne reproduis pas, mais je ne suis pas su de comprendre ce que tu as tenté de faire :) Bizarrement ça ne se produit qu'avec saisies_inserer_apres : ```php $saisies = saisies_inserer_apres($saisies, $nouvelle_saisie, 'saisie_qui_n_existe_pas'); ``` edi : mais bon c'est pas lié à cette PR, je note au passage.
maieul closed this pull request 10 months ago
Poster
Owner

C'est corrigé, mis en master + en v3 et c'est releasé.

C'est corrigé, mis en master + en v3 et c'est releasé.
This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.