DamienCassou

DamienCassou commented on pull request DamienCassou/overview#4

Fix highlighting the last heading.

Great job, thank you!

1 week ago

DamienCassou merged pull request DamienCassou/overview#4

Fix highlighting the last heading.

1 week ago

DamienCassou pushed to master at DamienCassou/overview

19bfcefc44 Merge pull request 'Fix highlighting the last heading.' (#4) from pteromys/overview:last-heading into master
0e23a4c14a Fix highlighting the last heading. When the last heading is supposed to be highlighted, we can't detect that by waiting for the heading after it to fail isAboveFirstThird() because there is no heading after it.
Compare 2 commits »

1 week ago

DamienCassou suggested changes for DamienCassou/overview#3

Support HTML5 sections.

Thank you very much for your hard work. This is taking shape.

1 month ago

DamienCassou commented on pull request DamienCassou/overview#3

Support HTML5 sections.

if the Outline class is part of its own file, you can probably move this code in `Outline.js`. Putting the responsibility in the right place is a way to simplicity.

1 month ago

DamienCassou commented on pull request DamienCassou/overview#3

Support HTML5 sections.

these changes could probably be part of a dedicated PR: "Introduce outline object". This would simplify this PR quite a bit and give me confidence.

1 month ago

DamienCassou commented on pull request DamienCassou/overview#3

Support HTML5 sections.

why are you changing this CSS? Is it related to supporting HTML5 sections which is the purpose of this PR? If not, I suggest taking this change out and making a separate PR out of it, with a rationale. If it is related, can you please explain (and maybe put in a separate commit)?

1 month ago

DamienCassou commented on pull request DamienCassou/overview#3

Support HTML5 sections.

why are you changing this function? Is it related to supporting HTML5 sections which is the purpose of this PR? If not, I suggest taking this change out and making a separate PR out of it, with a rationale. If it is related, can you please explain?

1 month ago

DamienCassou commented on pull request DamienCassou/overview#3

Support HTML5 sections.

this function could probably be extracted to a dedicated file and split into many small functions.

1 month ago

DamienCassou commented on pull request DamienCassou/overview#3

Support HTML5 sections.

can you please create a dedicated Outline.js file to contain this class? With a class comment. What about using the [class keyword](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes) to define the class?

1 month ago

DamienCassou commented on pull request DamienCassou/overview#3

Support HTML5 sections.

can you please comment each argument? I usually prefer to use a literal object when a function has several arguments, e.g.,

1 month ago

DamienCassou commented on pull request DamienCassou/overview#3

Support HTML5 sections.

refactoring is good, but please make that a separate commit so I understand the code changes better. If a commit changes everything at once, the chances that I miss something important are greater. Moreover, if one day I have to revert something (e.g., support for HTML5 sections), I can still easily keep the refactorings if I want to.

1 month ago

DamienCassou pushed to try-arm-silex at mpdel/libmpdel