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

[feat] handle multi-get #31 #61

Merged
merged 1 commit into from
Mar 28, 2016
Merged

[feat] handle multi-get #31 #61

merged 1 commit into from
Mar 28, 2016

Conversation

Nebulis
Copy link
Collaborator

@Nebulis Nebulis commented Mar 27, 2016

No description provided.

class="form-control"
placeholder="a.b['id', 'name']">
<span class="input-group-addon action">
<button ng-if="$last" class="btn btn-primary" type="button" ng-disabled="!$ctrl.paths[$index]" ng-click="$ctrl.addPath()">
Copy link
Owner

Choose a reason for hiding this comment

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

$ctrl.paths[$index] pourrait être remplacer par path non?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Je peux ajouter un comm si tu veux

Copy link
Owner

Choose a reason for hiding this comment

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

Je suis pas sûr de voir le rapport car le pb dans l'issue que tu link c'est que le ng-model va écrire dans le mauvais scope, alors qu'ici ng-disabled ne fait que lire.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My bad, j'avais pas fait attention que tu pointais ng-disabled. Oui je suppose que path pourrait marcher mais j'ai préféré laissé l'autre notation pour plus de cohérence

Copy link
Owner

Choose a reason for hiding this comment

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

OK

@hgwood
Copy link
Owner

hgwood commented Mar 28, 2016

Problèmes d'UX que je rencontre en utilisant :

  • Si je sais que j'ai 4 chemins à ajouter, je ne peux pas faire 4x "+" puis remplir les cases parce que je peux pas faire "+" tant que je n'ai pas rempli toutes les cases. Par ailleurs, même si je le pouvais, le bouton "+" bougerait et ce serait fastidieux de cliquer dessus 4x d'affilé.
  • Si j'ai ajouté un chemin de trop, je suis coincé, car je ne peux pas supprimer le dernier chemin, et je ne peux pas non plus faire "Get" avec un chemin vide.

@hgwood
Copy link
Owner

hgwood commented Mar 28, 2016

Les boutons "+" et "-" sont pas super biens intégrés dans l'input. Essaye avec ça : http://getbootstrap.com/components/#input-groups-buttons-multiple

@Nebulis
Copy link
Collaborator Author

Nebulis commented Mar 28, 2016

Pour le point 1 -> pas pensé a ce usecase. J'ai ajouté le bouton au niveau du label.

Pour le point 2 -> je m'étais fait la remarque, il y a d'autres usecase ou c'est un peu chiant : je peux ajouter un bouton supprimer en face de tous les input et juste empecher la suppression s'il ne reste qu'un input

Pour le point 3 -> deja essayé mais il y a une différence de style de base entre les examples et l'app. J'ai fix pour que ca ressemble au max

@hgwood
Copy link
Owner

hgwood commented Mar 28, 2016

  1. Parfait
  2. Parfait
  3. ben apparemment tu as changé quelque chose et c'est mieux maintenant

Reste seulement quelques remarques esthétiques pour rester cohérent avec le reste de l'appli :

  • je mettrais plutôt "Add one" à la place de "+" et "Remove this" à la place de "-" comme texte dans les boutons
  • les 2 boutons devrait plutôt être de classe btn-link car ce ne sont pas des boutons importants
  • le bouton Add devrait plutôt être en btn-xs je pense

@Nebulis
Copy link
Collaborator Author

Nebulis commented Mar 28, 2016

Done

@hgwood
Copy link
Owner

hgwood commented Mar 28, 2016

Super !

Qu'est-ce que tu dirais de faire carrément disparaître le bouton Remove quand il y a un seul input affiché ? Ainsi tant qu'on utilise pas la fonctionnalité, elle ne prend pas d'espace.

@Nebulis
Copy link
Collaborator Author

Nebulis commented Mar 28, 2016

👍
J'ai vu un probleme dans le fichier HTML, j'en ai profité pour le corriger, pour voir les modif que j'ai apporté : L15, L22, L23

@hgwood
Copy link
Owner

hgwood commented Mar 28, 2016

Merci :)

@hgwood hgwood merged commit 3b65788 into hgwood:master Mar 28, 2016
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.

2 participants