Follow-up from "WIP: 2018 Developer Survey Report"
The following discussions from !10144 (merged) should be addressed:
-
@iamphill started a discussion: Should this be debounced?
All do
this.onScroll.bind(this)
& then you don't need to dovar self = SectionNavigation;
-
@iamphill started a discussion: (+2 comments) Just use
document.querySelector
. No need to get jQuery involved when its not needed. -
@iamphill started a discussion: (+1 comment) I'd very much prefer to not include this. We can achieve the same without this.
-
@iamphill started a discussion: So this is just to make vanilla JS scrollTo smooth scroll?
Why not just use jQuery
.animate({ scrollTop: ... })
& not include more JS? -
@iamphill started a discussion: (+2 comments) Do we really need another weight here? We are already including a lot of font weights so I'd prefer to not include another.
-
@iamphill started a discussion: (+1 comment) This is already included in the main stylesheet right?
-
@iamphill started a discussion: (+1 comment) Why are we re-assigning a lot more new variables? Shouldn't most of these already exist in the
variables.scss
file? -
@iamphill started a discussion: 39px
is very precise - why not40px
? -
@iamphill started a discussion: (+3 comments) Lets not use
.js-*
classes in our CSS. We prefer to keep these to tell people there is JavaScript attached to the selector. If you need to, just add another class. -
@iamphill started a discussion: (+3 comments) Is it not this anyway?
-
@iamphill started a discussion: Same here re.
256
& scoping the variable like this. Also - its pretty similar to the one I previously mentioned, shouldn't we just use that? I know there is.07
difference, but thats not massive. -
@erica started a discussion: (+2 comments) Project expectations...
@luke @mattatrenet this needs to be removed from the in office/remote toggle display to and moved to the high versus low performing toggle display below
-
@erica started a discussion: My ideas and opinions...
@luke @mattatrenet remove from this toggle view and add to the high/low toggle view below