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

Add Pafat theme #623

Merged
merged 6 commits into from
Sep 26, 2014
Merged

Add Pafat theme #623

merged 6 commits into from
Sep 26, 2014

Conversation

plopoyop
Copy link

Proposition d'un thème pour FreshRss.
Testé sous Chromium et Firefox.
Testé avec les versions 0.7.3 à 0.8.0

@aledeg
Copy link
Member

aledeg commented Sep 18, 2014

Peux tu nettoyer les fichiers SVG de ce qui n'est pas nécessaire?
Voir #450

@plopoyop
Copy link
Author

J'avais pas vu qu'ils avaient été changés entre temps.
Je m'en occupe.

@aledeg
Copy link
Member

aledeg commented Sep 18, 2014

Si tu n'as pas d'icones particulières, tu peux les enlever car il y a un répertoire contenant les icones par défaut.
Il faut n'inclure que celles que tu veux changer.

Optimisation SVN
Utilisation du template par défaut.
@plopoyop
Copy link
Author

J'ai supprimé les icones identiques aux icones par défaut.
J'ai copié les icones optimisées et je les ai modifiées pour que les couleurs correspondent aux miennes.

J'ai supprimé le fichier template pour utiliser le fichier par défaut. (je n'avais pas vu qu'il y avait des choses qui avaient changées en 0.8).

@marienfressinaud
Copy link
Member

J'ai enfin pris le temps de regarder ! Déjà merci pour la proposition, le thème est très bien :)

J'ai un certain nombre de remarques (des petits détails à corriger) :

  • Les éléments <button /> sont légérement plus petits que les autres. Ça se voit bien notamment sur la page "gestion des abonnements", le bouton pour ajouter un flux. Un min-height: 29px; sur les buttons devrait être suffisant.
  • La croix de fermeture des notifications ne se voit pas (ou légèrement au survol de la zone de clic)
  • Il faudrait répercuter certains changements fait dernièrement sur ton thème (voir 097703f#diff-a1d544149f8311ab66cc3cd6b5a010a8R918)
  • Il y a un léger bug d'affichage dans la gestion des flux, partie "Identification". La partie "mot de passe" est décalée par rapport à l'identifiant.
  • Je trouve les boutons un peu petit pour un usage sur mobile. Si ça te convient je ne serai pas pénible à ce niveau-là ;)

Pour le moment je n'ai rien de plus à ajouter :)

@marienfressinaud
Copy link
Member

Petit détail visuel : ligne 381 (https://github.com/plopoyop/FreshRSS/blob/dev/p/themes/Pafat/pafat.css#L381) il faudrait que tu te bases sur le base.css (https://github.com/marienfressinaud/FreshRSS/blob/dev/p/themes/base-theme/base.css#L259). Càd le .as-link au même niveau que le span et réduire le padding à 22px pour ces deux-là (changement fait à l'instant donc tu ne pouvais pas en tenir compte avant ^^)

@plopoyop
Copy link
Author

J'ai fait les modifications :
min-height:29px sur les btn.
j'ai supprimé l'icone close.svg quand j'ai fait le dernier nettoyage, j'avais pas retrouvé à quoi elle servait, dans le doute, je l'avais gardé. Avec celle par défaut, c'est good.
C'est bon pour les stats
J'avais pas vu le décalage, merci, c'est mieux d'avoir une 2eme paires d'yeux.
J'ai remis le as-link comme le thème par défaut même si pour le moment, il y a une petite inconstance entre la dropdown de configuration et celle de "marquer comme lu", je ne savais pas trop comment les rendre identique sans faire un truc crade....
Pour la taille en mobile, je pense que je m'en occuperai en V2 du thème. J'ai des détails à corriger je pense mais rien de grave, d'où mon pull pour la 0.8.

Merci :)

@marienfressinaud
Copy link
Member

J'avais pas vu le décalage, merci, c'est mieux d'avoir une 2eme paires d'yeux.

Il y a toujours le décalage chez moi…

J'ai remis le as-link comme le thème par défaut même si pour le moment, il y a une petite inconstance entre la dropdown de configuration et celle de "marquer comme lu", je ne savais pas trop comment les rendre identique sans faire un truc crade....

.dropdown-menu > .item > a {
    padding: 0 25px;
    line-height: 2.5em;
    color: #666;
    font-size: 0.8rem;
}
.dropdown-menu > .item > span,
.dropdown-menu > .item > .as-link {
    padding: 0 22px;
    line-height: 2em;
}

semble pas trop mal.

Dernier petit détail, dans le .dropdown-menu:after il serait mieux de changer la couleur des borders en les mettant à #aaa (comme les bordures des .dropdown-menu).

@plopoyop
Copy link
Author

Les éléments du menu "marquer comme lu" on une écriture "plus épaisse" que ceux de la dropdown de configuration. Je me demande si ça ne serait pas mieux de faire directement :
.dropdown-menu > .item > a,
.dropdown-menu > .item > span,
.dropdown-menu > .item > .as-link {
padding: 0 22px;
line-height: 2.5em;
color: #666;
font-size: 0.8rem;
}

J'avais raté le détails des .dropdown-menu:after.
Le décalage existe en effet sous FF, c'est le problème de ne vérifier qu'avec un navigateur.

Je m'occupe de tout ça.

@marienfressinaud
Copy link
Member

En faisant ce que tu proposes il y a un décalage dans le dropdown des flux (colonne de gauche) pour le lien "marquer comme lu". De plus les span à l'intérieur des dropdown paraîtront plus gros que les liens, d'où la séparation initiale.

@marienfressinaud
Copy link
Member

Et effectivement les as-link ont une police plus épaisse, c'est d'autant plus visible sous Webkit apparemment. J'essaye de m'en charger mais je ne vois pas trop la propriété qui donne cet effet.

@plopoyop
Copy link
Author

Je me demande si ça n'est pas le :
font-size: 0.8rem;

@marienfressinaud
Copy link
Member

Il y a de ça, je pense que tu peux l'appliquer dessus, mais j'ai l'impression qu'il y a encore quelque chose d'autre ^^

@plopoyop
Copy link
Author

Je ne suis toujours pas complètement convaincu mais je n'ai pas trouvé la raison de la différence entre les textes.
J'ai fait les autres corrections en attendant.

@marienfressinaud
Copy link
Member

Ok pas de soucis, je m'occupe du reste pour intégrer dans FRSS ;)

@marienfressinaud marienfressinaud merged commit d847d67 into FreshRSS:dev Sep 26, 2014
marienfressinaud added a commit that referenced this pull request Sep 26, 2014
@marienfressinaud
Copy link
Member

Merged :)

@plopoyop
Copy link
Author

Merci :)

@marienfressinaud
Copy link
Member

J'ai juste corrigé un petit problème de marge et modifié "FreshRss" par "FreshRSS" ;)

@marienfressinaud
Copy link
Member

Pour le "Marquer comme lu" un peu différent je m'en chargerai pour les prochaines versions. Là je veux sortir la 0.8 aujourd'hui et pas envie de me prendre la tête dessus pour le moment ^^

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

4 participants