ctubbsii commented on code in PR #398:
URL: https://github.com/apache/accumulo-website/pull/398#discussion_r1320909988


##########
pages/downloads.md:
##########
@@ -12,8 +12,8 @@ var updateLinks = function(mirror) {
 };
 
 var mirrorsCallback = function(json) {
-  var htmlContent = '<div class="row"><div class="col-md-3"><h5>Select an 
Apache download mirror:</h5></div>' +
-    '<div class="col-md-5"><select class="form-control" 
id="apache-mirror-select">';
+  var htmlContent = '<div class="row align-items-center mb-3"><div 
class="col-3"><h5>Select an Apache download mirror:</h5></div>' +
+    '<div class="col-5"><select class="form-select" 
id="apache-mirror-select">';  

Review Comment:
   ```suggestion
       '<div class="col-5"><select class="form-select" 
id="apache-mirror-select">';
   ```



##########
pages/downloads.md:
##########
@@ -85,26 +85,29 @@ available in a future update to this site.
 {% for srcbin in srcbinArray %}
 {% assign lnkFile = 'accumulo-' | append: linkVers | append: '-' | append: 
srcbin | append: '.tar.gz' %}
 {% assign lnkSuffix = '/accumulo/' | append: linkVers | append: '/' | append: 
lnkFile %}
-<div class="row btn-group" style="margin-left: 20px; margin-bottom: 5px; 
display: block">
-  <div class="col btn-group">
+<div class="row" style="margin-left: 20px; margin-bottom: 5px;">
+  <div class="btn-group col-auto">
     <a {{btnDownloadStyle}} href="{{closerLink}}{{lnkSuffix}}" 
link-suffix="{{lnkSuffix}}">{{lnkFile}}{{glyphSave}}</a>
-  </div><div class="col btn-group">
-    <a {{btnSigStyle}} 
href="{{downloadsLink}}{{lnkSuffix}}.asc">ASC{{glyphLock}}</a>
-    <a {{btnHashStyle}} 
href="{{downloadsLink}}{{lnkSuffix}}.sha512">SHA{{glyphLock}}</a>
+    <div class="btn-group">
+      <a {{btnSigStyle}} 
href="{{downloadsLink}}{{lnkSuffix}}.asc">ASC{{glyphLock}}</a>
+      <a {{btnHashStyle}} 
href="{{downloadsLink}}{{lnkSuffix}}.sha512">SHA{{glyphLock}}</a>
+    </div>
   </div>
 </div>
 {% endfor %}
-<div class="row btn-group-sm" style="margin: 20px">
-  <a {{btnDocStyle}} 
href="{{site.baseurl}}/release/accumulo-{{linkVers}}">Release Notes</a>
-  <a {{btnDocStyle}} 
href="https://github.com/apache/accumulo/blob/rel/{{linkVers}}/README.md";>README</a>
-  <a {{btnDocStyle}} href="{{site.baseurl}}/docs/2.x">Online Documentation</a>
-  <a {{btnDocStyle}} 
href="https://github.com/apache/accumulo-examples";>Examples</a>
-  <a {{btnDocStyle}} href="{{site.baseurl}}/docs/2.x/apidocs3">Java API</a>
+<div class="row" style="margin: 20px;">
+  <div class="btn-group-sm">

Review Comment:
   This extra div creates a single column inside the row div. Previously, the 
intent was to have each `<a>` element be a separate column within the row.



##########
pages/downloads.md:
##########
@@ -85,26 +85,29 @@ available in a future update to this site.
 {% for srcbin in srcbinArray %}
 {% assign lnkFile = 'accumulo-' | append: linkVers | append: '-' | append: 
srcbin | append: '.tar.gz' %}
 {% assign lnkSuffix = '/accumulo/' | append: linkVers | append: '/' | append: 
lnkFile %}
-<div class="row btn-group" style="margin-left: 20px; margin-bottom: 5px; 
display: block">
-  <div class="col btn-group">
+<div class="row" style="margin-left: 20px; margin-bottom: 5px;">
+  <div class="btn-group col-auto">
     <a {{btnDownloadStyle}} href="{{closerLink}}{{lnkSuffix}}" 
link-suffix="{{lnkSuffix}}">{{lnkFile}}{{glyphSave}}</a>
-  </div><div class="col btn-group">
-    <a {{btnSigStyle}} 
href="{{downloadsLink}}{{lnkSuffix}}.asc">ASC{{glyphLock}}</a>
-    <a {{btnHashStyle}} 
href="{{downloadsLink}}{{lnkSuffix}}.sha512">SHA{{glyphLock}}</a>
+    <div class="btn-group">
+      <a {{btnSigStyle}} 
href="{{downloadsLink}}{{lnkSuffix}}.asc">ASC{{glyphLock}}</a>
+      <a {{btnHashStyle}} 
href="{{downloadsLink}}{{lnkSuffix}}.sha512">SHA{{glyphLock}}</a>
+    </div>

Review Comment:
   This still looks substantially different from what it was before, and I'm 
not sure if it has the same behavior as before. Originally, it was two button 
groups adjacent to one another, as if two columns in the same row. Here, you've 
changed it to a button group nested inside another, and the row effectively has 
one column now (which obviates the value of having a row).
   
   I think it should be:
   
   ```suggestion
     </div><div class="btn-group">
         <a {{btnSigStyle}} 
href="{{downloadsLink}}{{lnkSuffix}}.asc">ASC{{glyphLock}}</a>
         <a {{btnHashStyle}} 
href="{{downloadsLink}}{{lnkSuffix}}.sha512">SHA{{glyphLock}}</a>
   ```
   
   Where this matters is when you make the window size very small. This makes 
the buttons wrap in a very specific way that keeps the validation buttons (ASC, 
SHA) together, and situated just beneath the link to the file (rather than 
forcing the user to scroll), but without allowing them to easily wrap, creating 
a column of buttons mangled together with no logical groupings at all.



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to