From mboxrd@z Thu Jan 1 00:00:00 1970 From: andy at warmcat.com (Andy Green) Date: Thu, 28 Jun 2018 05:45:38 +0800 Subject: [PATCH v5 2/6] cgit.js: line range highlight: introduce javascript In-Reply-To: References: <153001215740.12484.13344875445662026876.stgit@mail.warmcat.com> <153001232465.12484.1444048603463034115.stgit@mail.warmcat.com> Message-ID: <7dcb5079-8248-8494-038f-214bbc8908b3@warmcat.com> On 06/28/2018 02:02 AM, Jason A. Donenfeld wrote: > Hi Andy, > > I'm super hesitant about the Pandora's box that introducing javascript > implies, but perhaps there's no use in fighting the future. The "fighting the future" bus left the station ten years ago when github was founded. Worrying about JS is just fighting the past. > A few notes: > > - Your js needs the copyright line like the rest of the project. > - Rather than awkwardly namespacing global methods, wrap everything > inside a big anonymous function. OK. > - 1.5s is way too long for an animation. In fact, I'm not sure what an > animation communicates at all, and perhaps we could get rid of it. We can't apply the highlight until the page completes its load event, because the vertical layout is subject to be adjusted by image loads etc right up until then. It looked nicer to me to have the highlight "become apparent" in the case the lines had already been visibly rendered without it. > - Setting colors from inside js is a big no-no. Instead, add and > remove classes from elements, and leave the styling to css. OK. > Also, can this be done from pure css? For example, certainly > highlighting one line in pure css based on the #anchor is possible, > via css link selectors. Maybe you are thinking something I'm missing, but there are three unrelated s there that happen to be rendered vertically aligned. They're not on a single despite it may look like that. -Andy > Jason >