Produits Entr'ouvert » Combo

Le mockup affiche cette information entre parenthèses et sans point final. Il affiche aussi une information quand aucun créneau n'existe.

Frédéric Péters a écrit :

Le mockup affiche cette information entre parenthèses et sans point final. Il affiche aussi une information quand aucun créneau n'existe.

C'est fait dans le patch juste ne dessus, en prenant en compte ce qui a été défini dans #19452

J'ai peut-être raté un bout, mais pourquoi une méthode __str__ en plus pour DaySlot alors qu'il y a déjà un __repr__ ? Et dédans l'utilisation de l'objet DateFormat alors que self.date_time est déjà un objet datetime et on peut appeler strftime dessus.

Vire les espaces dans autour de "-"

if (tmp[-1].date_time - tmp[0].date_time) != ((step - 1) * offset):

Juste lire

if (tmp[-1].date_time - tmp⁰.date_time) != ((step - 1) * offset):

Ici m'effraie.

Je me dis que dans le code il y aura des commentaires pour accompagner.

(et je vérifie et non)

     font-size: 13px;

nope nope nope. Ce calendar.css emmerde déjà les choses, n'obligeons pas les thèmes à contourner cette propriété; le souhait est d'être plus petit que la taille par défaut, disons ok, mais que ça passe par un pourcentage.

    def get_first_available_slot(self, offset, min_duration):
        """Search the first available slots
        but more importantly contiguous ones

Le commentaire ne semble pas tout à fait correspondre au nom de la fonction; le commentaire parle de retourner plusieurs slots.

Sur la forme, il me semble qu'il aurait été équivalent d'écrire :

required_contiguous_slots = ... # c'est peut-être équivalent au step mais je n'ai pas cherché ce que contien offset pour vérifier.
for i in range(len(self.slots) - required_contiguous_slots):
    if all([x.available for x in slots[i:i+required_contiguous_slots]):
        # got n contiguous slots, that's the one.
        return slots[i]
return None

(sur l'idée que les slots sont contigus, ce qui me semble être le cas à la lecture du patch)

+        return _('(No slot available)')

No available slots.

De manière générale, ce code qui joue avec ce qui est à afficher, ça aurait plutôt sa place dans le template, genre :

<span class="calinfo">
{% with calendar.get_first_available_slot as slot %}
({% if slot %}{% trans "Next available slot :" %}{{ slot|date:"whatever date format"}{% else %}{% trans "No available slots." %}{% endif %}
{% endwith %}
</span>

(la boucle qui se trouve dans get_info, qui se trouverait ici déplacée dans get_first_available_slot, pourrait sans doute aussi se trouver dans le template mais ce serait trop.)

font-size: 0.8em;

ça passe par un pourcentage → 80% ou whatever. Pas une autre unité.

"""Search the first available slots
but more importantly contiguous ones

Pas changé : le commentaire ne semble pas tout à fait correspondre au nom de la fonction; le commentaire parle de retourner plusieurs slots.

required_contiguous_slots = self.min_duration.seconds / self.offset.seconds

Pas besoin de calculer ça à chaque itération.

{% trans "Next available slot :" %}

Il y avait un piège, pas d'espace avant : en anglais.

assert '(Next available slot :June 13' in page.content

Écrire ce test devrait être suffisant pour se dire qu'il y a un truc à corriger.

{% trans "Next available slot: " %}

Non, on ne pose pas des espaces de fin de ligne dans les chaines à traduire, c'est trop facile ensuite de les rater. L'espace doit être dans le template entre {% trans "Next available slot: " %} et {{ slot.date_time...

    def get_first_available_slot(self):
        """Return the first available slot

Comme le commentaire n'apporte rien qu'une répétition du nom de la méthode, il ne sert vraiment pas. Pour le garder faut le prolonger avec de l'info supplémentaire, genre "return the first available slot that has enough consecutive available slots to be allowed for booking".

À rebaser pour ne pas utiliser calendar.css. → inclure <style>.calinfo { font-style: italic; font-size: 80%; } dans le template de la cellule.

Ok, ajouté dans ce patch.

Pas du tout, tu l'as ajouté en dur en attribut de l'élément alors que je disais qu'il fallait taper ça dans une balise <style>.

<style>.calinfo { ...

Vraiment.

Pas style="...".

Pas non plus <style>h2 > .calinfo.

Le raisonnement derrière c'est avoir le sélecteur avec la plus petite priorité/spécificité possible.

Ok.