Skip to content
This repository was archived by the owner on May 5, 2021. It is now read-only.

[WIP] color : parse and render odoo theme class into css style in the VDOM #469

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sebgeelen
Copy link
Contributor

@sebgeelen sebgeelen commented Nov 9, 2020

Needed to be able to re enable the Qunit test about theme color picker in odoo

@sebgeelen sebgeelen added the wip Currently being worked on label Nov 9, 2020
@sebgeelen
Copy link
Contributor Author

Discord Context :

Sébastien (sge), Today at 10:54 AM

@david Monjoie (dmo) je peut "ovverider" le default parsing engine de tout les plugins sans devoir pour autant overrider ler parser un par un ? par exemple la, j'aimerait faire un OdooXmlDomParsingEngine qui va surcharger le XmlDomParsingEngine mais je ne sais pas comment l'utiliser dans les parser sans les surcharger eux aussi.

David Monjoie (dmo), Today at 10:55 AM

@sébastien (sge) oulah t'es en train de faire quoi là ? x)

Sébastien (sge), Today at 10:56 AM

ben, je m'y prend surement pas bien du coup vu t'a réaction, mais j'essaye de transformer une classe en cssStyle au parsing mais je ne sais pas sur quel noeud ça va arriver, ça peut etre n'importe quel noeud

David Monjoie (dmo), Today at 10:57 AM

explique ton cas
ah tu fais les trucs des couleurs là ?

Sébastien (sge), Today at 10:58 AM

oui
<span class="bg-o-color1">abc</span> doit devenir (en VDOM) <span style="background-color: var(--o-color1)">abc</span>
le renderer c'est déja ok

David Monjoie (dmo), Today at 10:59 AM

t'as fait comment pour le renderer ?

Sébastien (sge), Today at 10:59 AM

mais je sais pas si ça va etre sur un span, un b, un p, ou quoi

Sébastien (sge), Today at 11:00 AM

j'ai surcharger le DomObjectModifierRenderer

David Monjoie (dmo), Today at 11:02 AM

@sébastien (sge) ça va pas être simple pour ton truc je pense
car c'est bien ce que je craignais, on a éclaté le principe des modifiers au rendu
mais pas au parsing

Sébastien (sge), Today at 11:02 AM

c'est ça
ou l'inverse en fait

David Monjoie (dmo), Today at 11:03 AM

et j'ai bien peur que ce ne soit pas simple à faire

Sébastien (sge), Today at 11:03 AM

enfin y en a un qui est facile mais pas l'autre

David Monjoie (dmo), Today at 11:04 AM

au rendering on a des NodeRenderer et des ModifierRenderer
on n'a pas ce concept au parsing
et on devrait

Nicolas Bayet (nby), Today at 11:05 AM

sauf avec la solution que les modifiers devienent des noeuds
just sayin'
^^

David Monjoie (dmo), Today at 11:05 AM

true mais ça change pas le problème que c'est pas un petit fix quoi

Nicolas Bayet (nby), Today at 11:05 AM

true

David Monjoie (dmo), Today at 11:05 AM

et en vrai non ça change pas
c'est les formats qui deviendraient des noeuds dans ton truc
ici c'est Attributes

Nicolas Bayet (nby), Today at 11:06 AM

ah

David Monjoie (dmo), Today at 11:06 AM

tu vas pas faire un conteneur avec les attributs quand même ?

Nicolas Bayet (nby), Today at 11:06 AM

non
:p

David Monjoie (dmo), Today at 11:06 AM

^^

Nicolas Bayet (nby), Today at 11:06 AM

note que ca pourrait

Sébastien (sge), Today at 11:06 AM

oui ça pourait
c'est comme si on utilisait que la moitier de la spec XML :p

David Monjoie (dmo), Today at 11:06 AM

ah ah

Nicolas Bayet (nby), Today at 11:07 AM

non ca serait bizare, ca voudrait dire que tous les noeuds enfants se trouve avoir les attributs

David Monjoie (dmo), Today at 11:07 AM

c'est pas impossible c'est sûr mais bon je vois pas trop le gain perso :p

Nicolas Bayet (nby), Today at 11:07 AM

indeed

David Monjoie (dmo), Today at 11:07 AM

les formats je vois le gain c'est pour être plus proche de l'html
mais les attributs ça devient un peu foufou :p

Nicolas Bayet (nby), Today at 11:07 AM

en effet

David Monjoie (dmo), Today at 11:07 AM

bon donc 2 solutions seb

  1. implémenter le split NodeParser, ModifierParser
  2. un hAcK dEgUeU

Sébastien (sge), Today at 11:08 AM

donc une seule solution

David Monjoie (dmo), Today at 11:08 AM

bah ça dépend du timing
fin oui quand on connait le timing y'en a qu'une qui est valide à chaque fois
mais suivant le timing c'est pas la même :p

Sébastien (sge), Today at 11:09 AM

pour moi c'est pas un breaking bug ( parce que on a qqch qui marche sur master)
donc si on a pas le temps de le faire proprement

David Monjoie (dmo), Today at 11:10 AM

tu as raison je pense

Sébastien (sge), Today at 11:10 AM

autant laisser comme maintenant

David Monjoie (dmo), Today at 11:10 AM

en effet ce serait dommage de faire un hack juste pour faire passer un test
alors que la feature marche

Sébastien (sge), Today at 11:10 AM

surtout que même si les gens utilise la méthode qui marche maintenant et qu'on change, ça restera retro compatible

Sébastien (sge), Today at 11:11 AM

et les var(color) seront transfromé et class="color" dès un edit

David Monjoie (dmo), Today at 11:11 AM

attends non y'aun truc qui me fait peur
oui justement
est-ce qu'on gère bien d'éditer un dom qui a les anciennes classes ?

Sébastien (sge), Today at 11:11 AM

oui

David Monjoie (dmo), Today at 11:11 AM

est-ce qu'on va bien reconnaitre la couleur ?
comment ça se fait ?

Sébastien (sge), Today at 11:11 AM

ah non
non, ça en effet ça va marchouiller

Sébastien (sge), Today at 11:12 AM

on poura quand même éditer les couleurs et tout
mais la classe va rester

David Monjoie (dmo), Today at 11:12 AM

ah ah oui je vois

Sébastien (sge), Today at 11:12 AM

mais vu que style est plus fort, visuelement ça va marcher

David Monjoie (dmo), Today at 11:13 AM

déplaces le bug en low priority

Sébastien (sge), Today at 11:13 AM

après faudra faire gaffe quand on ferra la vrai bonne solution
de garder le style si on a les 2

David Monjoie (dmo), Today at 11:13 AM

p-e que quand on tombera dessus on le descendra en wishlist je ne sais pas encore
de garder le style si on a les 2
@sébastien (sge) bah oui et non
car il faudra bien corriger ce qu'on a fait avec la version actuelle
^^'

Sébastien (sge), Today at 11:14 AM

ça ca se corrigera tout seul
parce que la version actuelle c'est ce qui aura dans le VDOM
et donc le renderer corigera automatiquement
mais au parsing faudra faire la distinction si on a les 2
( ce qui en vrai ne devrai jamais arriver )
sauf si tu édite un website créé en 14.0 et que tu change la couleur en master

David Monjoie (dmo), Today at 11:16 AM

mais non je comprends pas...

  1. vieille page:

    abc

  2. parsing + rendering:

    abc

  3. edit actuel:

    abc

  4. edit futur: si tu conserves le style comme tu veux faire, tu ne vas jamais parvenir à corriger le dom généré en 3

Sébastien (sge), Today at 11:16 AM

pour le moment le rendering ne va pas changer la classe en style

David Monjoie (dmo), Today at 11:17 AM

ok le 2 est faux
mais le 3 pas

Sébastien (sge), Today at 11:17 AM

oui, mais le cas numéro 3 c'est ce que je disait. Si on a les deux, on devra garder le style
par garder le style je veu dire

David Monjoie (dmo), Today at 11:18 AM

ok j'ai mal lu ce que t'avais écrit en fait

Sébastien (sge), Today at 11:18 AM

metre le style dans la class hein

David Monjoie (dmo), Today at 11:18 AM

my bad
oui c'est ça
je croyais que tu voulais conserver style et class au cas où c'est ce que le mec a voulu faire (ce qui est pas idiot mais on se tirerait une balle dans le pied à cause de la version actuelle de l'éditeur qui aura justement généré des cas comme ça involontairement)
mais ce que tu disais c'était de considérer style > class et d'updater la classe
j'avais pas compris

Sébastien (sge), Today at 11:19 AM

oui c'est ça
c'était en effet pas clair
bref je fait quoi du coup ? je met de coté ?

David Monjoie (dmo), Today at 11:20 AM

déplaces le bug en low priority
p-e que quand on re-tombera dessus on le descendra en wishlist je ne sais pas encore

Sébastien (sge), Today at 11:22 AM

je vais commit mes change actuel et créer un PR in progress pour garder le context

@sebgeelen sebgeelen changed the title [INPROGRESS] color : parse and render odoo theme class into css style in the VDOM [WIP] color : parse and render odoo theme class into css style in the VDOM Nov 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
wip Currently being worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant