regardscitoyens / the-law-factory

Track the french law-making process
https://www.lafabriquedelaloi.fr
GNU Affero General Public License v3.0
75 stars 15 forks source link

[wip] rewrite amendements rendering without d3 or svg #128

Closed njoyard closed 8 years ago

njoyard commented 8 years ago

This PR is a complete rewrite of the amendments rendering, no longer using d3 or svg. Rendering is done by AngularJS using the template, directly.

Rendering is much faster, and window resize is much smoother now (as the bulk of it is done by the browser, we only resize containers).

I tried to reproduce all existing features ; it is not 100% complete yet, the following is missing:

Also it would be nice to move amdt details loading to the api service and render it using a template.

This PR makes autorefresh very easy also, as we just have to reload the json data, put it in the angular scope and let it do its thing.

Let me know what you think.

njoyard commented 8 years ago

Sorting/grouping is also missing. I'm working on it :)

RouxRC commented 8 years ago

And also probably:

I may still forget a couple things :)

Apart from this, that is what we envisionned when saying this module had to be rewritten from scratch, so thanks! One thing though: if you look at the branch refactor-v2, @fmassot started refactoring all the api/services/utils/directives part, I guess it would be best to integrate your code directly in his new paradigm for easier future merge (one of the objective will be that grouping/sorting or selection will be bound to url arguments so we can have permalinks for all).

RouxRC commented 8 years ago

ah, and one good example to try to check the speed would be the mariage pour tous bill here with its 5000 amendments: https://www.lafabriquedelaloi.fr/amendements.html?loi=pjl12-349&etape=02_1%C3%A8relecture_assemblee_hemicycle

Note how the grouping is handled to keep the sorting interesting: it is filled as a snake column after column from up to bottom then bottom to up etc

fmassot commented 8 years ago

The refactoring seems very interesting as it will close issue https://github.com/regardscitoyens/the-law-factory/issues/106

Moreover the code is far better :)

However, as this is a complete rewrite, I wonder if it will be not better to fully integrate this module in angular. As you may have notice, the current code is bad as it mixes angular stuff (the scope) with jquery outside angular.

Finally there is still some work to do, some features as the one's you describe are missing, small features too (like click outside of any amendements will reset legend filters) so this work will not be merge immediately in master branch.

I would imagine a merge of your work after a merge of branch refactor-v2 (which is a sort of "big clean") in master. What do you think @RouxRC ? On my side, I will work on a merge of refactor-v2 this weekend.

njoyard commented 8 years ago

I'll be working on the refactor-v2 branch as of now. I'll try to mimic existing features as closely as possible (legend actions are already implemented FYI, but need a rewrite, too).

There's one thing I don't see how to reproduce without heavy resize code, though: the "snake-column" filling. I'm using CSS flex here and switching it to columns is easy, but there's no way to have the filling direction change at every wrap.

Also, that's my first experience with Angular so don't hesitate to point rookie mistakes.

RouxRC commented 8 years ago

I guess the snake column filling can be handled via the sorting right after the regular sort, by reordering things to adapt to the current length of the div whenever multiple lines are required

It might be a bit tricky but that should be doable with some algo on the indices of the array using modulos, similarly to what was done here and there : https://github.com/regardscitoyens/the-law-factory/blob/master/public/js/modules/mod2.js#L264-L271 https://github.com/regardscitoyens/the-law-factory/blob/master/public/js/modules/mod2.js#L289-L296

njoyard commented 8 years ago

Yeah, I have a good idea on how to do it :) My point was that we'd have to have this logic in a window resize handler, instead of letting the browser do everything.

FYI I'm quite close to something fully functional on the refactor-v2 branch (probably ready tomorrow as I have other things to do in the afternoon).

RouxRC commented 8 years ago

Not sure how the whole is handled but if the whole rendering of the amendments divs is binded to variables in angular's scope, just trigger a reordering of the amendments array on resize could suffice to force redrawing accordingly.

njoyard commented 8 years ago

See #129