Demon theme refactor

[csakiistvan]

Első lépésként betettem az alap sass fileokat, és a sok cssből a kimenetben csak egy style.css file lesz, ezt hívja meg a smink is. Szükséges lesz még további szeparálás a _drupalhu_style.scss fájlból, de kezdetnek úgy gondoltam megteszi.

[csakiistvan]

@aboros mivel ez nagyreszt front endet érintő változás, kérlek nézz rá te is

[tanarurkerem]

uh... biztos kell ez? De ha már compass-ra át akarunk térni, akkor én első körben egy olyan compass fájl egyveleget látnék szívesen, ami a már meglévő css-eket gyártja le, úgy, hogy ha megdiffelem (a szóközökön és egyéb üres karaktarakan kívül) nincs különbség.

A css tömörítést pedig rábíznám a Drupalra.

De felmerül bennem a kérdés, hogy biztos kell ez nekünk? Szemeim előtt az lebeg, hogy a "fejlesztőkörnyezet kialakítása" leírásunk még egy újabb fejezettel fog bővülni a compass miatt.

[csakiistvan]

Ezt azt csinálja, ugyanazt a stílust adja vissza mint a sok css külön külön, plusz lényegi munkát nem akartam beletenni. Az hogy milyen css megy ki mellékes szvsz

[aboros]

"a "fejlesztőkörnyezet kialakítása" leírásunk még egy újabb fejezettel fog bővülni a compass miatt." igen! akárhogy is kerül ez be, a kapcsolódó readme részek is legyenek benne a PRben szerintem.

de általánosságban mindenképp kell ez, manapság ezek nélkül front endet fejleszteni igazán nem szokás, jó okkal. sok compass plugin van amiknek hasznát vehetnénk és a fileok szétszedése logikai egységekre is jó gyakorlat szerintem.

ami gondot látok, hogy ez így ebben a formában használja a compasst amit a gépemen talál vagy ha nem talál elhasal. erre ugye az a megoldás, hogy npm csomagkezelővel telepíted a "projektbe lokálisan" a compasst, meg az egyéb ilyeneket és akkor npm install és feláll ugyanaz ami máshol is. azért kell ez, mer különben folyton para lesz vele, hogy az én compassom meg a többieké hanyas verziójú (és ugyan így minden más ilyennel is, bootstrap, ilyen-olyan grid systemek vagy bármi plugin amit használunk majd.) a dokumentáció pedig annyival frissül, hogy miután klónoztad a repót, add ki a smink gyökerében az npm install parancsot. (persze telepítsd a node.jst előtte:))

[csakiistvan]

Oké, beleteszek egy Bundlert

[aboros]

ja, az! mindig keverem ezt a sok frontendOPs cuccot :)

[tanarurkerem]

Ez ok. A kerdes ezt hogyan teszteled? Amit javasoltam az egy egyszeru diff-fel tesztelheto. Amit te csinaltal, az csak "szemmel". Ezert javasoltam azt, h dobjuk ki azt a lepest amit drupal egyebkent is megcsinal.

2014.12.28. 9:56 ezt írta ("Csáki István" notifications@github.com):

Ezt azt csinálja, ugyanazt a stílust adja vissza mint a sok css külön külön, plusz lényegi munkát nem akartam beletenni. Az hogy milyen css megy ki mellékes szvsz

— Reply to this email directly or view it on GitHub.

[csakiistvan]

@tanarurkerem Lehet láma dolog, de mivel ugyanazokat a fájlokat dolgozzuk fel, őszintén kétlem hogy bármi diff lehetne ami hibát okozhatna, és igen, csak szemre kattintgattam végig.

[csakiistvan]

A sima css fájlok sorainak száma összesen 2214, a compass által generált css sorszáma, line commentek nélkül pedig 2285 sor. Így sorra mindenképp megvagyunk, a többlet egyéb változtatásokból lesz.

[tanarurkerem]

Félreértesz. Az issue most azt, hogy compassra álljunk át. Ezt most megtetted, de hogyan teszteljük? Ránézünk és, ha jó akkor jó. Te is tudod, hogy simán előfordulhat, hogy valamelyik rejtett oldalon előjöhet egy hiba, mert a css szabályok sorrendje nem lesz ugyan az.

Ha először betennénk egy olyan compass-t, ami ugyan azokat a css-eket gyártaná le, ugyan oda, akkor viszonylag egyszerűen lehetne ellenőrizni, hogy jó-e a compass átalakítás. Fogom és legenerálom, majd azt mondom, hogy "git diff -w" és ha az nem mutat változást, akkor biztos lehetek benne 100%-ig, hogy nem lesz gond. Igazából abban lehetek biztos, hogy a compass ugyan azt a végeredményt adja, legyen az bármilyen jó, vagy vacak, már az eredmény, mert most ugye a "compass átalakítás" - így egyben - jóságát vizsgáltam..

Leszedhetném a PR-edet, és pikk-pakk ellenőrizhetném, és már nyomnám is rá a merge-re.

Így most ránéztem, és azt gondolom, hogy erre, így én nem fogok merge-t nyomni, mert nem látom át, hogy mi változott. És szerintem bárki más se fogja megmondani, hogy jó-e, még Te se vagy benne száz százalékig biztos.

De ez is csak egy szempont.

[csakiistvan]

hát nem nagyon értem amit kérsz mert ez tök logikátlan lenne ha minden fájl külön scss lenne és ugyanazt a fájlt generálná le amit az élesen is megtalálsz, hisz pont azért készült ez a PR, de ott vannak a fájlok a baseben, lehet diffelni ha akarod nem mindegyiket ugyanugy hivjak ez igaz, pl a 960 nemtudommi simán csak _grid.scss lett, a style.css meg _drupalhu.scss.

[tanarurkerem]

de egy darab css-t gyárt. Tök mindegy egyébként, egy sör mellett majd hosszabban.

[tanarurkerem]

csak azért írtam, hogy lásd, hogy valaki foglalkozott a pr-el, de nem tudta mergelni.

[aboros]

szerintem a diffelhetőség mellett amúgy is hasznos lenne, ha a css aggregálást a drupalra hagynánk. mivel a csseket is elkommitoljuk így a jövőben is kevesebb konflikt lesz, ha rendesen szeparált cssek mennek be nem egy már uglified cucc.

[csakiistvan]

a css már nem tömörített, lásd https://github.com/csakiistvan/drupal.hu/commit/b85cf4c00556fdb58f356f2ce3be757742951e78

[aboros]

igen de így most nem diffelhető mer szét van szedve fileokra. :) sajnos nem gondolkodtunk ennyire előre, hogy előbb compass sass kéne bundlerrel és aztán szedni szét darabokra. de felőlem így is maradhat én tartózkodok. nekem elég, ha szemre ugyan az :)

[aboros]

meg most ez akadni fog a #157 el.

[csakiistvan]

@tanarurkerem @aboros kérés szerint visszakerült a régi hierarhia, és bekerült a readmebe is a leírás.

[csakiistvan]

A #160 al is összeakad majd, megcsinálom újra, ezen nem múlik.

[tanarurkerem]

wow, köszi. Felvettem a repodat csakiistvan néven

git remote add csakiistvan [repourl]

Átmentem a demon-refactor-ra

git checkout demon-refactor

majd kiadtam a következő parancsot:

git diff -b -w --ignore-space-at-eol --ignore-blank-lines dev **/*.css

Na ezzel már meg tudtam nézni én is. :) Bár sok változtatás van, amik teljesen "lényegtelenek" (híres utolsó mondatok), de találtam azért pár érdekességet. Fontossági sorrendben:

search.css legfontosabb talán, hogy megváltozott a kép elérési útvonala.

 .view-multi-index-kereso .comment {
-  background: url("../images/comment_icon.png") no-repeat scroll 11px 11px #00588D;
+  background: url("../../images/comment_icon.png") no-repeat scroll 11px 11px #00588D;
   padding-left: 53px;
 }

frontpage.css bekerült a .view-field-title kiválasztó, de nem látom, hogy lett volna törölve kód, tehát ez nem összevonás, hanem valami plusz

+body.front .view-link-ajanlo .views-field-title,
 body.front .view-link-ajanlo .view-footer {
   margin-top: 10px;
 }

átkerült máshova ez a szabály:

.views-slideshow-controls-text .views-slideshow-controls-text-pause {
  display: none;
}

A többi módosítás főleg coding standard hibák javítása, meg egy csomó üres szabály is bekerült. Ami talán érdekes lehet, hogy az /* LTR */ megjegyzések átkerültek más sorba, ami nem tudom, hogy probléma, mert lehet, hogy valami (pl. tömörítő) használja ezeket a megjegyzéseket. (mindjárt utána kutatok)

[tanarurkerem]

Na megnéztem, az nem gond, mert csak a szerkesztőknek van, hogy tudják melyik szabályra kell odafigyelni, mert az van az -rtl-es fájlban is. Itt egy kis írás a témában: http://jwilson3.postach.io/right-to-left-theming-in-drupal-with-directional-scss

[tanarurkerem]

Ha a fenti három dolog rendben van, akkor mergelhető.

[tanarurkerem]

ja és még valami. A README.md tele van olyan üres sorokkal, amikben szóközök vannak, meg van egy csomó sor végi szóköz is. Lécci állítsd be a szerkesztődet, hogy ezeket lecsapja, mert tök zavaró.

[tanarurkerem]

Néztem ezt a compass-t működés közben.

1. Docker

Összeraktam egy Compass Docker Container-t. Annak a használata szerintem egyszerűbb, bár window-on nem biztos, hogy megy, de ugye mindenki Linuxot vagy Mac-et használ. :)

https://github.com/tanarurkerem/compass

Szóval elvileg csak egy alias, ha már telepítve van a Docker és akkor lehet használni a compass-t vidáman. Próbáld ki, és h a tetszik, akkor mehet a readme-be.

1. Builder

Ez a rész nekem nem kerek a README.md-ben.

A --deployment után nekem nem ment a build exec compass, mert a builder ugye nem tudja, hogy --deployment van és máshol keresi a compass-t, de nem lehet neki megadni, hogy --deployment, mert az exec ezt a kapcsolót nem ismeri. Valamit rosszul csináltam? (egy friss vm-ben lehet tesztelni ezt a hibás működést)

Valamint ha builder mellett maradunk, akkor a .gitignore-ba adjuk hozzá a vendor könyvtárat.

[csakiistvan]

@tanarurkerem a css kéréseket megcsináltam a readme-t stormal és egy MD editorral is megnéztem, de nem láttam azt amire te gondolsz. Dockerem nincs, így az utolsó kommentedet nem tudom tesztelni.

[tanarurkerem]

A README.md-t azért majd nézze meg valaki. de nem prio, mert compass-t ugy is lehet telepíteni, ha valaki nagyon akarja, nem érdemes nekünk megismételni a compass-nál található leírást.