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

Modification des titres et sous-titres de contenu "in place" #5804

Closed
wants to merge 3 commits into from

Conversation

Arnaud-D
Copy link
Contributor

@Arnaud-D Arnaud-D commented May 22, 2020

Fix #5796.

Ça ressemble à ça :

image

Et quand on clique pour afficher le formulaire :

image

image

Contrôle qualité

En tant que simple visiteur ou simple membre (ni auteur, ni staff) :

  • regarder l'affichage correct des titres et sous-titres sur tout le site (tutoriels, articles, billets, forums, et pages classiques (dans le profil, les listes de contenus, etc.)
  • vérifier l'absence des boutons de modification des tutos/articles/billets quand on est publié ou en bêta ;
  • vérifier qu'on se prend les erreurs adéquates quand on tente de se servir malicieusement des formulaires ;

En tant qu'auteur ou staff :

  • vérifier la présence des boutons de modification ;
  • vérifier que le bouton de la description change entre le petit stylo et "Ajouter un sous-titre" selon la présence ou non d'un sous-titre ;
  • vérifier le bon affichage/désaffichage des formulaires quand on joue avec les boutons modifier/annuler ;
  • vérifier la fonctionnalité nominale des formulaires (bon changement du titre et de la description, messages de confirmation, etc.)
  • vérifier la fonctionnalité des formulaires pour les erreurs quand on a les droits pour s'en servir (slug invalide, etc.)

@Arnaud-D Arnaud-D changed the title Modification des titres et sous-titre de contenu "in place" Modification des titres et sous-titres de contenu "in place" May 22, 2020
@coveralls
Copy link

coveralls commented May 22, 2020

Coverage Status

Coverage increased (+0.06%) to 86.685% when pulling 3e93606 on Arnaud-D:titre_inplace into fe854d9 on zestedesavoir:dev.

@Arnaud-D Arnaud-D force-pushed the titre_inplace branch 2 times, most recently from f41c424 to 38d719f Compare May 22, 2020 21:40
@Arnaud-D
Copy link
Contributor Author

C'est assez avancé pour être testable, si certains voudraient dors et déjà faire quelques retours.

@Arnaud-D Arnaud-D marked this pull request as ready for review May 24, 2020 10:46
@Arnaud-D
Copy link
Contributor Author

C'est prêt à être testé et revu. Je suis preneur de conseils et revues sur le JavaScript et CSS, je suis un débutant dans ces deux choses là.

<img src="{{ content.image.physical.tutorial_illu.url }}" alt="">
{% endif %}
{{ content.title }}
<div class="title-show" id="title-show">
Copy link
Contributor

@A-312 A-312 May 24, 2020

Choose a reason for hiding this comment

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

@A-312
Copy link
Contributor

A-312 commented May 24, 2020

Un icône crayon au lieu d'un texte évitera peut être de surcharger l'interface, ensuite la description ne respecte pas le padding vertical, c'est troublant.

Faut-il vraiment passer par un input et un bouton enregistrer/annuler. Certaine application adopte plutôt un contenteditable et de l'autosave (ou à minima un bouton valider sans texte (=> un icône)) ?

L'autre alternative en gardant cette aspect avec un formulaire et un input est d'afficher un bloc en position:absolute sous l'élement :
image

De plus adopter une "fenêtre" de modification comme ça, pourrait permettre de glisser un bouton pour ne pas valider la partie en question voir même de glisser le bouton supprimer/déplacer dedans pour les petits écrans.

@Arnaud-D
Copy link
Contributor Author

Arnaud-D commented May 24, 2020

Je vais corriger pour l'histoire de div dans le h1. Pour le problème de padding, je ne sais pas corriger sans que ce soit bizarre et sans y passer des jours.

Le système à boutons Enregistrer/Annuler est inspiré de GitHub et me paraît suffisant dans un premier temps.

Je ne suis pas contre les systèmes de mise à jour automatique, mais je le verrai plutôt dans un deuxième temps. Déjà, c'est au-delà de mes compétences aujourd'hui. J'ai aussi l'impression que ces systèmes peuvent avoir des comportements pas si intuitifs que ça (typiquement le système de tags de GitHub, où il faut cliquer en dehors du menu déroulant, puis attendre, pour savoir si ça a été pris en compte...). Il faut aussi qu'ils puissent gérer les erreurs correctement. Ça fait pas mal de dév en plus de la solution de cette PR.

Pour l'icône, je suis entièrement pour, mais dans une autre PR. Je ne l'ai pas fait pour les licences, parce que je n'ai pas réussi à trouver une solution potable à mes yeux sans y passer des heures. On peut le faire dans un autre ticket et l'utiliser pour tous les champs et modales de ce type.

Pour le bloc fixe, je ne suis pas sûr de comprendre. Ma PR ne traite que les cas du titre et sous-titre de contenu, pas de partie. Tu veux t'en servir aussi pour les titres de section, partie, chapitre ?

@A-312
Copy link
Contributor

A-312 commented May 24, 2020

image

A la limite en respectant ce qui est déjà fait ☝️ <button class="btn btn-grey ico-after edit" /> ou même un seul bouton "Editer le titre & la description", est-ce vraiment utile de séparer les deux ?

@A-312
Copy link
Contributor

A-312 commented May 24, 2020

Pour le bloc fixe, je ne suis pas sûr de comprendre. Ma PR ne traite que les cas du titre et sous-titre de contenu, pas de partie. Tu veux t'en servir aussi pour les titres de section, partie, chapitre ?

Je n'avais pas vu que c'était pour les titres, je pensais que c'était pour les parties.


Si ça peut t'aider, si je devais faire ça, je ferais comme ça dans les grandes lignes :

<div id=headtitle>
  <h1><img /> Titre </h1> <button class="btn btn-grey ico-after edit" />
  <h2 class="subtitle" itemprop="description">description</h2>
</div>
<form id=form_headtitle style="display:none;">
  <input name=title />
  <input name=description />
</form>
$('#headtitle button').click(() => $('#headtitle, #form_headtitle').toggle('form_headtitle'))

Je cacherais/remplacerais les éléments HTML du titres et descriptions (en les mettant dans une même div, pour à avoir seulement à cacher la div parent). Gérer séparément l'affichage de l'édition permet de ne pas à avoir à gérer le rendu normal (hors-édition) et le rendu pendant l'édition en même temps (ça évite que certaines règles écrasent d'autres et la sémantique sera respecté). Après tu es libre de reproduire l'apparence visuelle que tu veux pour form_headtitle.

@Arnaud-D
Copy link
Contributor Author

J'aime bien l'idée de reprendre l'icône du menu de gauche, je n'y avais pas pensé du tout. Pas convaincu pour faire un seul champ pour les deux ; ce n'est pas d'ailleurs pas tellement compatible avec l'idée de faire un truc en mode contenteditable plus tard.

Franchement, si tu te sens de faire de façon aboutie tes propositions, fais-le. J'ai retesté à l'instant des choix plus normaux pour le balisage, mais je m'en sors pas avec le CSS pour avoir un rendu acceptable.

J'ai pas envie d'y passer des jours. Donc c'est soit dans un second temps (donc une autre PR), soit dans très longtemps (ou jamais).

@A-312
Copy link
Contributor

A-312 commented May 28, 2020

Pas convaincu pour faire un seul champ pour les deux ; ce n'est pas d'ailleurs pas tellement compatible avec l'idée de faire un truc en mode contenteditable plus tard.

C'est dommage qu'on ne peut pas rendre modulable le formulaire en fonction des champs présents, je veux dire prendre en compte les cas :

  • Titres présents ;
  • Descriptions présentes ;
  • Titres et descriptions présentes.

Ca simplifierait grandement la tâche en fonction des changements/besoins futur.


Pour le CSS que tu demandes (après pour la couleur, je te laisse faire A mon avis le line-height, color, background-color, border-color, font-size doivent peut-être être modifié ).

image

#form_headtitle {
  margin-bottom:20px;
}

#form_headtitle .hline img {
  width: 50px;
  height: 50px;
  border: 1px solid #cdd0d1;
  z-index:1;
}

#form_headtitle .hline button {
  line-height:32px;
  height:32px;
  color:black;
}

#form_headtitle .hline {
  position:relative;
  display: flex;
  align-items:center;
  margin-bottom:5px;
}
#form_headtitle .hline.title::after {
  position:absolute;
  display:block;
  content: ' ';
  height:42px;
  border-left:10px transparent solid;
  box-sizing: border-box;
  width:100%;
  border-bottom: 1px solid #f8ad32;
  z-index:0;
}

#form_headtitle .hline.title input {
  align-items:center;
  /* Je ne suis pas sûr du padding/margin a appliquer : */
  margin-left:8px;
  padding:0 5px;
}
<form id="form_headtitle" style="">
  <div class="hline title">
    <img src="/media/galleries/9028/1753f5bd-9d3d-4b5f-bd64-71263be35a99.png.60x60_q95_crop.png" alt="" itemprop="thumbnailUrl">
    <input type="text" name="title">
    <button>Valider</button>
   </div>
  <div class="hline desc">
    <input type="text" name="description">
    <button>Valider</button>
  </div>
</form>

@A-312
Copy link
Contributor

A-312 commented May 28, 2020

Je viens de voir que j'ai tous mis dans le même formulaire, alors que tu veux laisser ça indépendant. Suffit juste de séparer les éléments et d'avoir un form indépendant :

image

@AmauryCarrade AmauryCarrade added the C-Front Concerne l'interface du site label Jun 18, 2020
@firm1
Copy link
Contributor

firm1 commented Jun 22, 2020

Je vois que la branche est pas mal en conflit. J'aurai voulu faire la QA, mais j'ai un doute si je peux partir sur l'état actuel.

@Arnaud-D
Copy link
Contributor Author

Je pensais que c'était assez prêt, mais en fait j'ai encore du travail sur :

  • le CSS, parce que c'est trop moche à mon goût ;
  • la structure du HTML, qui gêne pour faire un CSS propre.

@Arnaud-D
Copy link
Contributor Author

Je pense que la structure HTML est plutôt bien. Pour l'instant, elle est implantée pour la vue brouillon seulement, mais il faudrait l'adapter pour la vue publiée, puisque le nouveau CSS sera essentiellement commun.

Par contre, je galère sur le CSS. Mes compétences progressent, mais trop lentement pour avancer à un rythme agréable.

@AmauryCarrade j'ai pushé mon statut actuel, si tu veux tenter quelque chose, ou si tu as des conseils pour m'éviter de pédaler dans la semoule. ;-)

@Arnaud-D Arnaud-D force-pushed the titre_inplace branch 2 times, most recently from f333e15 to 6f09107 Compare August 7, 2020 20:59
@A-312
Copy link
Contributor

A-312 commented Sep 12, 2020

L'exemple de CSS ci dessus n'est pas valide ?

@Arnaud-D
Copy link
Contributor Author

Amaury fait une refonte globale du CSS du site (#5922 et #5835 notamment). On a décidé d'attendre qu'il finisse avant de faire le boulot ici pour éviter d'avoir à faire des retouches.

@Arnaud-D Arnaud-D force-pushed the titre_inplace branch 2 times, most recently from 75a2b9b to b515479 Compare September 13, 2020 07:41
@Arnaud-D Arnaud-D force-pushed the titre_inplace branch 2 times, most recently from 773f018 to f7112a0 Compare October 6, 2020 20:50
@Arnaud-D Arnaud-D force-pushed the titre_inplace branch 2 times, most recently from 4089e65 to ae0a070 Compare October 12, 2020 06:43
@Arnaud-D Arnaud-D force-pushed the titre_inplace branch 3 times, most recently from ed2b214 to 6edaea1 Compare December 19, 2020 17:29
@Arnaud-D
Copy link
Contributor Author

Bon, j'ai rebasé suite à la PR #5922.

Bonne nouvelle : ce qui avait été fait n'est presque pas cassé. Mauvaise nouvelle : c'est toujours aussi bancal !

@AmauryCarrade je sais que tu as une bonne âme, viens en aide à cette PR. Il "suffit" de mettre ça en ordre. ^^

@AmauryCarrade
Copy link
Member

Je ne me risquerai pas à te donner une date vu ce que j'ai sur les messages et les correctifs de #5922, mais je vais jeter un œil quand je le pourrai :) .

@Arnaud-D Arnaud-D force-pushed the titre_inplace branch 2 times, most recently from 4696f56 to 52971b4 Compare February 22, 2021 21:50
@AmauryCarrade AmauryCarrade self-requested a review May 18, 2021 10:27
@Arnaud-D Arnaud-D added the S-Zombie Ticket ou PR oubliée label Mar 17, 2022
@Situphen
Copy link
Member

Fermé par décision de la réunion des devs

@Arnaud-D Arnaud-D deleted the titre_inplace branch March 9, 2024 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Front Concerne l'interface du site S-Zombie Ticket ou PR oubliée
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Modification du titre et du sous-titre d'un contenu "in place"
6 participants