jdaugherty commented on code in PR #425:
URL:
https://github.com/apache/grails-static-website/pull/425#discussion_r2766899169
##########
assets/javascripts/plugins-search.js:
##########
@@ -1,31 +1,65 @@
-const queryInputFieldId = "query";
-const mobileQueryInputFieldId = "mobilequery";
-const allPluginsContainerDivClass = "allplugins";
-const pluginContainerDivClassName = "plugin";
-const allPluginsHeadingLabelClassName = "allpluginslabel";
-const searchResultsDivClassName = "searchresults";
-const searchResultsHeadingLabelClassName = "searchresultslabel";
-const searchResultsLabelSelector = "h3." + searchResultsHeadingLabelClassName;
-const gitHubStarsSelector = "div.githubstar";
-const noresultsDivClassName = "noresults";
-
-const allPlugins = [];
-const elementsClassNames = [allPluginsContainerDivClass,
allPluginsHeadingLabelClassName];
-
-window.addEventListener("load", (event) => {
- const elements = document.querySelectorAll("div." +
allPluginsContainerDivClass + " ul > li.plugin");
+const queryInputFieldId = 'query'
+const mobileQueryInputFieldId = 'mobile-query'
+const allPluginsContainerDivClass = 'all-plugins'
+const pluginContainerDivClassName = 'plugin'
+const allPluginsHeadingLabelClassName = 'all-plugins-label'
+const searchResultsDivClassName = 'search-results'
+const searchResultsHeadingLabelClassName = 'search-results-label'
+const searchResultsLabelSelector = 'h3.' + searchResultsHeadingLabelClassName
+const gitHubStarsSelector = 'div.github-star'
+const noresultsDivClassName = 'no-results'
+const allPlugins = []
+const elementsClassNames = [allPluginsContainerDivClass,
allPluginsHeadingLabelClassName]
+
+// Tab navigation
+document.addEventListener('DOMContentLoaded', () =>{
Review Comment:
Space between `=>` and `{`
##########
buildSrc/src/main/groovy/website/model/documentation/Snapshot.groovy:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package website.model.documentation
+
+import groovy.transform.CompileStatic
+
+@CompileStatic
+class Snapshot implements Comparable<Snapshot> {
Review Comment:
I'm not suggesting we do this for this review, but what are your thoughts
about having a single implementation of this?
We seem to have copied the basic code for software version *everywhere*.
Should we finally publish a single library with a Software / Snapshot /
SoftwareVersion / GrailsVersion class? It would help centralize this logic and
make it easier to change if needed.
##########
assets/javascripts/plugins-search.js:
##########
@@ -1,31 +1,65 @@
-const queryInputFieldId = "query";
-const mobileQueryInputFieldId = "mobilequery";
-const allPluginsContainerDivClass = "allplugins";
-const pluginContainerDivClassName = "plugin";
-const allPluginsHeadingLabelClassName = "allpluginslabel";
-const searchResultsDivClassName = "searchresults";
-const searchResultsHeadingLabelClassName = "searchresultslabel";
-const searchResultsLabelSelector = "h3." + searchResultsHeadingLabelClassName;
-const gitHubStarsSelector = "div.githubstar";
-const noresultsDivClassName = "noresults";
-
-const allPlugins = [];
-const elementsClassNames = [allPluginsContainerDivClass,
allPluginsHeadingLabelClassName];
-
-window.addEventListener("load", (event) => {
- const elements = document.querySelectorAll("div." +
allPluginsContainerDivClass + " ul > li.plugin");
+const queryInputFieldId = 'query'
Review Comment:
Historically, I've only skipped semi-colons when I have a linter to confirm
I'm following the gotchas of
https://tc39.es/ecma262/multipage/ecmascript-language-lexical-grammar.html#sec-automatic-semicolon-insertion
It's probably ok in this case though since we're just checking in static js.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]