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

[Feature] Mode lecteur #671

Closed

Conversation

sandhose
Copy link
Contributor

@sandhose sandhose commented Jun 2, 2014

Q R
Correction de bugs ? non
Nouvelle Fonctionnalité ? oui
Tickets concernés #659

Rapidement implémenté un bouton "Mode lecture"... Encore à intégrer ça correctement question design (pour le moment, ce n'est qu'un simple bouton), et éventuellement rajouter un raccourci clavier (ESC pour revenir en mode normal, Ctrl+L ou je ne sais quoi pour entrer en mode lecteur...) + éventuellement masquer d'autres éléments (type zone de commentaire, footer...)

Autre idée: permettre le changement de taille de font, et eventuellement combiner cela avec le mode nuit (cf. #584 )

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 29d75eb on sandhose:feature-659-reader-mode into 17800b2 on zestedesavoir:integration.

@coma94
Copy link
Contributor

coma94 commented Jun 2, 2014

Il me semble justement que changer la taille de la font et son type (sérif/sans sérif) est prévu pour une future version :) (mais pas prioritaire).

On 2 juin 2014 22:45:13 UTC+02:00, Sandhose notifications@github.com wrote:

| Q | R

| ------------------------- |

| Correction de bugs ? | non
| Nouvelle Fonctionnalité ? | oui
| Tickets concernés | #659

Rapidement implémenté un bouton "Mode lecture"... Encore à
intégrer ça correctement question design (pour le moment, ce n'est
qu'un simple bouton), et éventuellement rajouter un raccourci
clavier (ESC pour revenir en mode normal, Ctrl+L ou je ne sais quoi
pour entrer en mode lecteur...) + éventuellement masquer d'autres
éléments (type zone de commentaire, footer...)

Autre idée: permettre le changement de taille de font, et
eventuellement combiner cela avec le mode nuit (cf. #584 )
You can merge this Pull Request by running:

git pull https://github.com/sandhose/zds-site feature-659-reader-mode

Or you can view, comment on it, or merge it online at:

#671

-- Commit Summary --

  • added reader-mode class
  • added an (ugly) "Reader mode" button

-- File Changes --

M assets/css/main.css (2)
M assets/css/scss/_wide.scss (29)
M assets/css/scss/main.scss (2)
A assets/images/sprite@2x-s7500912c30.png (0)
D assets/images/sprite@2x-sdc8bfa9a21.png (0)
A assets/js/custom/reader-mode.js (20)
M templates/base_content_page.html (2)

-- Patch Links --

https://github.com/zestedesavoir/zds-site/pull/671.patch
https://github.com/zestedesavoir/zds-site/pull/671.diff


Reply to this email directly or view it on GitHub:
#671

@Alex-D
Copy link
Contributor

Alex-D commented Jun 2, 2014

Merci pour cette première PR, par contre je vais paraître chiant, mais le français est préféré à l'anglais pour les commits (la majorité en a décidé ainsi). Pas gênant, mais préférable pour la suite.

Pour le JS, globalement on se base sur jQuery ça serait cool si tu pouvais adapter ton code (sauf si tu es un radicaliste comme ThS :D)

En fait ton JS avec jQuery se résume à un simple

$('#readerModeBtn').on('click', function(e){
    $('.machin').toggleClass('reader-mode');
    e.stopPropagation();
    e.preventDefault();
});

ou quelque chose comme ça.

Je n'ai pas testé encore, mais ça me semble être un bon début :)

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 29d75eb on sandhose:feature-659-reader-mode into 17800b2 on zestedesavoir:integration.

@sandhose
Copy link
Contributor Author

sandhose commented Jun 2, 2014

Disons que je suis plutôt radicaliste face à jQuery :p (j'étais en train de me demander si je devais pas faire une PR en adaptant tout a du JS pur ^_^ )

Et j'ai fait cette structure en prévision d'une éventuelle amélioration de cette fonctionnalité...

On Mon, Jun 2, 2014 at 11:08 PM, Coveralls <notifications@github.com="mailto:notifications@github.com">> wrote:

Coverage remained the same when pulling 29d75eb on sandhose:feature-659-reader-mode into 17800b2 on zestedesavoir:integration.

Reply to this email directly or view it on GitHub.

@@ -58,7 +58,7 @@

SITE_ROOT = os.path.realpath(os.path.dirname(os.path.dirname(__file__)))

SITE_URL = 'http://127.0.0.1:8000'
SITE_URL = 'http://sandhose.fr:8000'
Copy link
Contributor

Choose a reason for hiding this comment

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

La SITE_URL en local, ce devrait être localhost en fait (marche en IPv4 et IPv6).

Copy link
Contributor

Choose a reason for hiding this comment

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

Disons que je suis plutôt radicaliste face à jQuery :p
Toi tu vas te faire un kopaing !

2014-06-02 23:15 GMT+02:00 SpaceFox notifications@github.com:

In zds/settings.py:

@@ -58,7 +58,7 @@

SITE_ROOT = os.path.realpath(os.path.dirname(os.path.dirname(file)))

-SITE_URL = 'http://127.0.0.1:8000'
+SITE_URL = 'http://sandhose.fr:8000'

La SITE_URL en local, ce devrait être localhost en fait (marche en IPv4
et IPv6).


Reply to this email directly or view it on GitHub
https://github.com/zestedesavoir/zds-site/pull/671/files#r13307656.

@sandhose
Copy link
Contributor Author

sandhose commented Jun 2, 2014

Whoops, my bad, j'étais persuadé d'avoir remis à zéro le fichier avant de commit... Sorry!

@Alex-D
Copy link
Contributor

Alex-D commented Jun 2, 2014

Je rajoute que 1 PR = 1 fonctionnalité.

Le mode nuit et compagnie ça viendra plus tard, au moins dans d'autres PR.

@Alex-D
Copy link
Contributor

Alex-D commented Jun 2, 2014

Encore un ajout :

Et j'ai fait cette structure en prévision d'une éventuelle amélioration de cette fonctionnalité...

La politique de dev du site veut qu'on code ce qui est nécessaire. On ne prévoit rien pour après : soit on le fait, soit on ne le fait pas.

Simplement parce que celui qui améliorera ne sera peut-être pas toi, il fera peut-être autrement et donc ça laisserait du code mort, ce qui est relativement dommage.

@@ -256,6 +260,9 @@
width: 22.5%;
border-bottom: none;

@include single-transition(width, $reader-mode-transition-duration);
overflow: hidden;
Copy link
Contributor

Choose a reason for hiding this comment

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

Pas fan de ce genre d'astuce non plus

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling b6c99e5 on sandhose:feature-659-reader-mode into 17800b2 on zestedesavoir:integration.

@Alex-D Alex-D added Front and removed Front labels Jun 2, 2014
@Alex-D
Copy link
Contributor

Alex-D commented Jun 3, 2014

Salut !

Finalement j'ai fait vraiment plus simple, plus factorisé/réutilisable et plus accessible ici : #684

Merci en tous cas pour ta PR, elle m'aura inspiré dans mon code (malgré jQuery, désolé pour ton amour propre :D).

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

5 participants