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

Squelette #9

Merged
merged 2 commits into from
Jan 16, 2017
Merged

Squelette #9

merged 2 commits into from
Jan 16, 2017

Conversation

DavidBruant
Copy link
Collaborator

Désolé, c'est un peu le dawa. Mais sans conséquence non plus.
Ne pas bloquer si rien n'est grave.

Copy link
Contributor

@thom4parisot thom4parisot left a comment

Choose a reason for hiding this comment

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

So far so good. I added a few comments but they are on the improvements edge rather than on the structural one.

return state;
}

fetch('./data/cedi_2015_CA.csv')
Copy link
Contributor

Choose a reason for hiding this comment

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

À paramétriser : ça peut être une action de reducer pour maitriser quand c'est téléchargé et quel fichier ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oui, on est dans le main.js pour le moment et j'essaye de précipiter le visuel.
Je rajoute une issue. https://github.com/dtc-innovation/dataviz-finances-gironde/issues/11

@@ -0,0 +1,57 @@
export default function remember(...args){
Copy link
Contributor

Choose a reason for hiding this comment

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

Pour le coup ça gagnerait à avoir un docblock avec un exemple d'utilisation pour capter à quoi ça sert/comment ça marche. Au pif je dirais :

import { remember } from './client/js/remember.js';

remember('toto', 'tata').then(() => console.log(localStorage.toto === 'tata'));

Copy link
Collaborator Author

@DavidBruant DavidBruant Jan 16, 2017

Choose a reason for hiding this comment

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

En fait, c'est pire que ça, il faut que je déplace remember dans son propre module NPM

https://github.com/dtc-innovation/dataviz-finances-gironde/issues/12

Et la doc d'usage sera dans le readme.


<script defer src="client/js/browserify-bundle.js"></script>

<link rel="stylesheet" href="https://rawgit.com/DavidBruant/better-defaults/master/defaults.css">
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to use than normalise.css?

https://github.com/stackcss/sheetify can help bundle CSS in JS and extract them out at bundling time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On peut peut-être utiliser normalize.
Mon better-default fait des choix tranchés genre box-sizing border-box et mettre les images de background au centre par défaut (plutôt que le coin en haut à gauche).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@DavidBruant
Copy link
Collaborator Author

C'est dur d'écrire en français, hein ? :-p

@DavidBruant DavidBruant merged commit bc9d6e9 into datalocale:master Jan 16, 2017
@thom4parisot
Copy link
Contributor

Pff, je m'en rends plus compte :-(

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.

2 participants