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

Fenêtre d'options #53

Merged
merged 18 commits into from
Apr 20, 2016
Merged

Fenêtre d'options #53

merged 18 commits into from
Apr 20, 2016

Conversation

WinXaito
Copy link
Collaborator

@WinXaito WinXaito commented Apr 9, 2016

Hello,

j'ai créer une ébauche pour une fenêtre d'option, encore rien de fonctionnel. Est-ce que ça te convient pour le moment ?
(J'ai prévu un peu large, style d'affichage, raccourcis, etc.)

@firm1
Copy link
Owner

firm1 commented Apr 9, 2016

Aie, je suis un peu embêté parce que tu as déjà pas mal avancé. Mais au niveau de la vue de gestion des options il est préférable d'utiliser des onglets pour tous ces choix plutôt que de passer par toutes ces setVisible(). ça sera vachement plus extensible et simple à utiliser.

@WinXaito
Copy link
Collaborator Author

Ah, si j'ai utilisé ça, c'est essentiellement pour une question de visuel et parce que j'ai souvent l'habitude de voir ça dans les logiciels que j'utilise au quotidien.

Cf: PhpStorm, IntelliJ, PyCharm, SolidWorks, eclipse, Visual Studio Community, etc...

Après ça m'est un peu égal, si tu veux que je change, je change ;)

@WinXaito
Copy link
Collaborator Author

zw_dev_3

Voilà une ébauche avec des onglets. Je trouve personnellement que c'est bien moins "jolie", mais après c'est effectivement plus simple à géré.

@firm1
Copy link
Owner

firm1 commented Apr 10, 2016

Je crois que c'est possible de reproduire le visuel que tu as fais avant, mais avec les onglets, en passant une couche de css dessus non ?

Une version améliorée de ce qui est proposé sur ce lien : http://stackoverflow.com/questions/29085983/create-vertical-tabs-in-tabpane-javafx

@WinXaito
Copy link
Collaborator Author

Hm, c'est à creuser.
Mais pour l'instant je me concentre sur les options, au pire j'importerai les AnchorPane dans l'autre, rien de très compliqué.

@firm1
Copy link
Owner

firm1 commented Apr 10, 2016

Tout à fais

@WinXaito
Copy link
Collaborator Author

On pourrais faire un petit récapitulatif des options à mettre (pour le moment) ? Voici ce que j'avais pensé:

Editeur:
  Apprence de rendu:
    Police d'écriture
    Taille d'écriture

Affichage:
  Thème (A voir si on proposera des thèmes dark par exmple)

Authentification:
  Serveur:
    Protocole
    Hôte
    Port

@firm1
Copy link
Owner

firm1 commented Apr 10, 2016

Apprence de rendu:

Je préfère qu'on ait pas à modifier le rendu, puisque l'idée c'est qu'on puisse garder le même rendu (police comprise) que celle du site.

Thème (A voir si on proposera des thèmes dark par exmple)

Je pense que ça serait un gros plus

Authentification:

Je ne suis pas sur que ces options soit vraiment utiles à l'utilisateur

De plus je vois bien une options pour :

  • Sauvegarder sa connexion au site pour pas qu'on le redemande à chaque fois que l'app est lancée si on veut download/upload
  • Masquer certains boutons (qu'on utilise pas souvent) de la barre d'outil d'édition (gras, italique, etc.)

@WinXaito
Copy link
Collaborator Author

Authentification:

Je ne suis pas sur que ces options soit vraiment utiles à l'utilisateur

De plus je vois bien une options pour :

Sauvegarder sa connexion au site pour pas qu'on le redemande à chaque fois que l'app est lancée si on veut download/upload
Masquer certains boutons (qu'on utilise pas souvent) de la barre d'outil d'édition (gras, italique, etc.)

Bonne idée, pour les paramètres du site, je suis parti du principe que ZdS était OpenSource et pouvais être installé par n'importe qui. ça peut-être utile pour faire des Tests en beta aussi.

Apprence de rendu:

Je préfère qu'on ait pas à modifier le rendu, puisque l'idée c'est qu'on puisse garder le même rendu (police comprise) que celle du site.

Alors la police et la taille d'écriture pour la rédaction ? Etant donné que certain utilisateur on peut-être leur préférence pour rédiger.

@firm1
Copy link
Owner

firm1 commented Apr 10, 2016

pour les paramètres du site, je suis parti du principe que ZdS était OpenSource et pouvais être installé par n'importe qui. ça peut-être utile pour faire des Tests en beta aussi.

Effectivement, mais peut-être alors dans un menu qui dit clairement que c'est "Avancé".

Alors la police et la taille d'écriture pour la rédaction ?

+10 pour les options de la rédaction

@WinXaito
Copy link
Collaborator Author

Je trouve que géré les configs avec des enum est bien plus simple, ton avis ?

@firm1
Copy link
Owner

firm1 commented Apr 10, 2016

Je ne suis pas sur de comprendre. Comment tu vois ça avec les enums ?

Le dim. 10 avr. 2016 17:44, WinXaito notifications@github.com a écrit :

Je trouve que géré les configs avec des enum est bien plus simple, ton
avis ?


You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#53 (comment)

@WinXaito
Copy link
Collaborator Author

tu peux regarder mon dernier commit

@firm1
Copy link
Owner

firm1 commented Apr 10, 2016

Avis positif.

Tu me diras quand tu penses avoir fini alors

@WinXaito
Copy link
Collaborator Author

J'ai une question, j'aimerais ajouter mainApp dans MdConvertController, mais je ne trouve pas ou MdConvertController est initialisé .. ?

@firm1
Copy link
Owner

firm1 commented Apr 10, 2016

Tu peux récupérer mainApp dans MdConvertController via mdBox.getMainApp().

@WinXaito
Copy link
Collaborator Author

Bon a partir de maintenant on peut géré l'écriture + taille dans l'éditeur (Pas la partie rendu).

Pour l'authentification, faudra peut-être voir ça dans une prochaine PR ?

Et sinon faudrait juste me dire les différentes écriture que tu veux.

@firm1
Copy link
Owner

firm1 commented Apr 15, 2016

Ah désolé, un peu pris cette semaine. Je vais faire une review de ton code. Et sinon pour

Pour l'authentification, faudra peut-être voir ça dans une prochaine PR ?

Tout à fais, je suis même pas sur que le back soit vraiment près pour ça.

Et sinon faudrait juste me dire les différentes écriture que tu veux.

Écriture en terme de font ?

@@ -5,6 +5,9 @@
import java.util.List;
import java.util.Optional;

import com.zestedesavoir.zestwriter.utils.Configuration;
import com.ziclix.python.sql.pipe.Source;
Copy link
Owner

Choose a reason for hiding this comment

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

Je crois que cet import est une erreur. Tu peux le supprimer

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Effectivement, soit une erreur soit un mauvaise import.

@WinXaito WinXaito mentioned this pull request Apr 18, 2016
@WinXaito
Copy link
Collaborator Author

Il y a un truc que je comprend pas avec les private Optional<Pair<String, String>> result;, car j'aimerais que si on a Username + password enregistré dans la config, on tente une connexion lors du démarrage de l'application.

Mais je ne sais pas quoi mettre comme premier paramètre dans new LoginService(...), en ayant juste 2 strings (Username + password).

Merci ;)

@firm1
Copy link
Owner

firm1 commented Apr 19, 2016

En fait, a regarder je me rend compte que le constructeur de LoginService n'est pas efficace.

Il vaudrait mieux le changer en public LoginService(String username, String password, ZdsHttp zdsUtils) pour qu'il soit plus accessible.

Du coup il faudrait modifier 2-3 trucs dans le MenuController. Je peux te pousser ça si tu veux.

@WinXaito
Copy link
Collaborator Author

Je veux bien que tu t'occupes de modifier LoginService pour adapter le constructeur, car j'ai un peu de la peine a comprendre cette structure.

Ta moyen de faire un commit directement sur ma branche ?

@firm1
Copy link
Owner

firm1 commented Apr 19, 2016

Ouaip je vais te faire ça.

Le mar. 19 avr. 2016 15:40, WinXaito notifications@github.com a écrit :

Je veux bien que tu t'occupes de modifier LoginService pour adapter le
constructeur, car j'ai un peu de la peine a comprendre cette structure.

Ta moyen de faire un commit directement sur ma branche ?


You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#53 (comment)

@WinXaito
Copy link
Collaborator Author

Plutôt que de modifier le constructeur, tu n'as pas moyen de le surcharger ?

@firm1
Copy link
Owner

firm1 commented Apr 19, 2016

Je t'ai fais la PR qui va bien sur ton dépot. C'est plus cohérent de modifier le constructeur

@WinXaito
Copy link
Collaborator Author

Je dois encore mettre à jour en me basant sur la branche Master et ensuite je pense que c'est bon pour cette PR.

Je rajouterai la possibilité de supprimer la ToolBar dans une prochaine PR.

Pour me mettre à jour si je fais un git fetch upstream master et un git pull upstream master et corriger les conflits est-ce bon ?

@firm1
Copy link
Owner

firm1 commented Apr 19, 2016

Yep ça devrait etre bon oui.

Le mar. 19 avr. 2016 20:49, WinXaito notifications@github.com a écrit :

Je dois encore mettre à jour en me basant sur la branche Master et
ensuite je pense que c'est bon pour cette PR.

Je rajouterai la possibilité de supprimer la ToolBar dans une prochaine
PR.

Pour me mettre à jour si je fais un git fetch upstream master et un git
pull upstream master et corriger les conflits est-ce bon ?


You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#53 (comment)

@WinXaito
Copy link
Collaborator Author

Une fois que les checks on passé et que tu es OK, c'est bon pour Merge !

@firm1
Copy link
Owner

firm1 commented Apr 19, 2016

Pas encore visiblement, car le merge n'est toujours pas possible.

@WinXaito
Copy link
Collaborator Author

Voilà, je ne comprend pas trop pourquoi ce conflits est venu après coût. Tu as fais un push entre temps ?

En tout cas maintenant c'est bon !

@firm1
Copy link
Owner

firm1 commented Apr 19, 2016

c'est peut etre mon push effectivement :( desolay

Le mar. 19 avr. 2016 22:21, WinXaito notifications@github.com a écrit :

Voilà, je ne comprend pas trop pourquoi ce conflits est venu après coût.
Tu as fais un push entre temps ?

En tout cas maintenant c'est bon !


You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#53 (comment)

@WinXaito
Copy link
Collaborator Author

Pas de soucis, maintenant c'est bon ! Et j'ai vu lors du conflit le bouton de contrôle de nouvelle version, c'est cool ça !

@firm1
Copy link
Owner

firm1 commented Apr 20, 2016

Bon boulot.

J'ai un bug au niveau de la fenêtre des options. Quand je clique sur le bouton et que je sélectionne ma police, la police n'est pas modifié dans le bouton (on voit toujours "Arial - 14").

J'avoue que la disposition sous forme de liens qui rendre les boites visibles ou non selon qu'on clique dessus me trouble un peu. Je veux bien repasser dessus une fois que ça sera mergé pour passer sur un style à base d'onglets avec stylisation de ceux ci.

@WinXaito
Copy link
Collaborator Author

Je comptais refaire directement une PR après pour améliorer le système d'option.

Pour le bug, si tu valides les options et que tu réouvre le panneaux cest bon. Je comptais régler sa dans la prochaine PR. Mais si tu veux je le fais sur celle ci (bien quelle commence a devenir illisible)

Pour le style, sa met égale. Voir si on ne peut pas mettre un bon coup de CSS pour arriver a un rendu similaire a sa ( que personnellement j'aime bien).

@firm1
Copy link
Owner

firm1 commented Apr 20, 2016

Bon bah si tu prévois de corriger le bug, je merge ça et j'attends la prochaine PR. Comme je release dimanche, ça serait cool que l'on ait épuré ça d'ici là.

Merci encore pour tes efforts

@firm1 firm1 merged commit 255ad24 into firm1:master Apr 20, 2016
@WinXaito
Copy link
Collaborator Author

Oui bien entendu, je vais faire mon possible.
Et au passage tu as tester l'auto connexion ? (en cochant "Restant connecté" lors du login ? Sa te convient ?)

@firm1
Copy link
Owner

firm1 commented Apr 20, 2016

Et au passage tu as tester l'auto connexion ? (en cochant "Restant connecté" lors du login ? Sa te convient ?)

C'est nickel ça. Par contre, va falloir trouver un moyen de ne pas stocker le mot de passe sur le disque. J'essaye de trouver une façon propre de faire ça, sans grand moyen.

@WinXaito
Copy link
Collaborator Author

Jai penser de garder le tocken de connexion ? Valable quelques mois

@firm1
Copy link
Owner

firm1 commented Apr 20, 2016

Yep c'est une idee

Le mer. 20 avr. 2016 10:04, WinXaito notifications@github.com a écrit :

Jai penser de garder le tocken de connexion ? Valable quelques mois


You are receiving this because you modified the open/close state.

Reply to this email directly or view it on GitHub
#53 (comment)

@WinXaito WinXaito deleted the feature_options branch April 20, 2016 18:17
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.

None yet

2 participants