[GitHub] [camel-website] AemieJ opened a new pull request #444: CAMEL-15335: update the search layout design

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] AemieJ opened a new pull request #444: CAMEL-15335: update the search layout design

GitBox

AemieJ opened a new pull request #444:
URL: https://github.com/apache/camel-website/pull/444


   ### Search Layout Design
   * Version is separated from the title of the section to keep a neater look and easier to understand the search result.
   * The borders have been removed as they don't fit in with the theme of the website.
   * On hovering a particular link, the whole section will give a shade of #f5f5f5 providing a highlighting effect.
   * Algolia attribute has been at the bottom of the search result.
   
   ![search-result](https://user-images.githubusercontent.com/44139348/88996674-7b013880-d30b-11ea-9507-ae401508bfbb.png)
   
   * Changed the look for **Nothing found** for no results found on a search query.
   
   ![no-result](https://user-images.githubusercontent.com/44139348/88996679-7d639280-d30b-11ea-8468-dd8dda2b469c.png)
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] zregvart commented on a change in pull request #444: CAMEL-15335: update the search layout design

GitBox

zregvart commented on a change in pull request #444:
URL: https://github.com/apache/camel-website/pull/444#discussion_r463986800



##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -375,13 +375,33 @@ body {
 
 #search_results {
   top: 3.38rem;
-  background: #fffe;
+  background: #fff;

Review comment:
       ```suggestion
     background: var(--navbar-background);
   ```

##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -390,15 +410,45 @@ body {
   padding-right: 0.5rem;
 }
 
+#search_results div.result {
+  padding: 0.25rem;
+  pointer-events: none;
+}
+
+#search_results div.result:hover {
+  background: var(--color-smoke-50);
+}
+
+#search_results div.section {
+  display: flex;
+}
+

Review comment:
       This was causing scrollbars to show
   
   ```suggestion
   
     #search_results dl {
       margin: 0;
     }
   ```

##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -412,9 +462,22 @@ body {
   padding-top: 0.3rem;
 }
 
+#search_results div.footer-search {
+  display: inline-flex;
+  float: right;
+  margin-right: 0.75rem;

Review comment:
       `float` doesn't make sense in flex layout, better to use padding vs margin so not to introduce scrollbars
   
   ```suggestion
     padding: .75rem;
     width: 100%;
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] aashnajena commented on pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

aashnajena commented on pull request #444:
URL: https://github.com/apache/camel-website/pull/444#issuecomment-667595711


   This implementation looks very neat and I like how the versions are displayed.
   
   I have one doubt - is it common to highlight the entire section when hovering over one search result? It's a little confusing as to what I'm clicking - one search result or the entire section (even with the underline detail). I think it would be much clearer if we just highlight the search result user is hovering on, or at least provide some clear indication of what the user is clicking on (the background grey and underline grey are very similar, and on Chrome, only the heading gets underlined).
   
   Another observation is that when you move from one search result to its next within the same section, the grey background flickers because there's some space between two results which does not have the same hover property.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] aashnajena commented on pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

aashnajena commented on pull request #444:
URL: https://github.com/apache/camel-website/pull/444#issuecomment-667657881


   This PR needs to wait. I'm observing that [this additional commit](https://github.com/algolia/docsearch-configs/commit/77634d58dab0f46f7dfc020a7b7fae1e9b390fe2#diff-a63c497e1db6a7dcc07ef35536d19967) made by an Algolia maintainer on our config file is causing some problems. @zregvart I have opened a [PR on the Algolia repository](https://github.com/algolia/docsearch-configs/pull/2133) to fix these issues.
   
   ![image](https://user-images.githubusercontent.com/32356795/89121123-b8b3bc00-d4d9-11ea-9199-1335c86bc8a9.png)
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] AemieJ commented on pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

AemieJ commented on pull request #444:
URL: https://github.com/apache/camel-website/pull/444#issuecomment-667659120


   @aashnajena yes, I already observed it. I am currently focusing on dealing with it without making change to the config file.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] AemieJ commented on pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

AemieJ commented on pull request #444:
URL: https://github.com/apache/camel-website/pull/444#issuecomment-667659479


   @zregvart this is why I suggested pagination for search results within the JIRA issue 'cause even if I show 5 of the results for some cases, it's going to overflow and the only time the scrollbar doesn't appear is when 3 results are presented on the search result but in my opinion, 3 results are not a convenient or suitable number to show to the user for a search query.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] aashnajena commented on pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

aashnajena commented on pull request #444:
URL: https://github.com/apache/camel-website/pull/444#issuecomment-667660367


   @AemieJ notice that blogs are not appearing as they should. They are getting grouped by categories. I don't think we should update the design layout without first fixing the flaws on the config side, it would be a little hacky. PR #445 along with PR #2133 on the Algolia repo will fix the underlying issue, and then we can work on top of that to improve the layout.
   
   Also, since different versions of the same page appear as multiple search results, shouldn't we be showing more search results to make sure we are including all relevant entries? Sorry if I'm missing out on something that's already been discussed. I don't understand what's the issue with the current format.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] AemieJ commented on pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

AemieJ commented on pull request #444:
URL: https://github.com/apache/camel-website/pull/444#issuecomment-667661044


   @aashnajena I am only talking about the default value being Documentation problem. What do you exactly mean by blog not being display as it is? Do the blogs need to be represented as -
   ```
   Blog
   Category
   Title
   ```
   If it's the case as above then the hierarchy needs to be reordered in the case of blogs.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] aashnajena commented on pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

aashnajena commented on pull request #444:
URL: https://github.com/apache/camel-website/pull/444#issuecomment-667661586


   The difference between current and expected behaviour is visible on the snapshot on PR #445


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] AemieJ commented on pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

AemieJ commented on pull request #444:
URL: https://github.com/apache/camel-website/pull/444#issuecomment-667685425


   @zregvart in the algolia doc-search, Aashna created a PR, if that is merged and PR #445 is merged as well, then I can continue working on this PR.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] AemieJ removed a comment on pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

AemieJ removed a comment on pull request #444:
URL: https://github.com/apache/camel-website/pull/444#issuecomment-667661044


   @aashnajena I am only talking about the default value being Documentation problem. What do you exactly mean by blog not being display as it is? Do the blogs need to be represented as -
   ```
   Blog
   Category
   Title
   ```
   If it's the case as above then the hierarchy needs to be reordered in the case of blogs.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] zregvart commented on a change in pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

zregvart commented on a change in pull request #444:
URL: https://github.com/apache/camel-website/pull/444#discussion_r465355877



##########
File path: package.json
##########
@@ -8,7 +8,6 @@
     "build:sitemap": "gulp sitemap",
     "build": "run-s build:*",
     "preview": "run-s preview:*",
-    "preview:antora": "antora --clean --fetch antora-playbook.yml",

Review comment:
       I don't think this was meant to be included here.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] zregvart commented on a change in pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

zregvart commented on a change in pull request #444:
URL: https://github.com/apache/camel-website/pull/444#discussion_r468195526



##########
File path: antora-ui-camel/src/js/vendor/algoliasearch.bundle.js
##########
@@ -46,22 +46,22 @@
         cancel.style.display = 'block'
         index
           .search(search.value, {
-            hitsPerPage: 10,
+            hitsPerPage: 5,
           })
           .then((results) => {
             const hits = results.hits
             const data = hits.reduce((data, hit) => {
               const d = {}
               d.url = hit.url
               var section = hit.hierarchy.lvl0
-              if (hit.hierarchy.lvl6 !== null) section = section + ' [' + hit.hierarchy.lvl6 + ']'
+              if (hit.hierarchy.lvl6 !== null) section = section + '/' + hit.hierarchy.lvl6
               var breadcrumbs = Object.values(hit.hierarchy)
                 .slice(1)
                 .filter((lvl) => lvl !== null)
-                .join(' » ')
+                .join(' / ')
 
               d.breadcrumbs = ((breadcrumbs !== '') ? breadcrumbs : section)
-              d.snippet = hit._snippetResult.content.value + '...'
+              d.snippet = hit._snippetResult.content.value.split('"').join('') + '...'

Review comment:
       Seems that `hit._snippetResult` can be undefined, are we using `d.snippet`?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] AemieJ commented on a change in pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

AemieJ commented on a change in pull request #444:
URL: https://github.com/apache/camel-website/pull/444#discussion_r468396127



##########
File path: antora-ui-camel/src/js/vendor/algoliasearch.bundle.js
##########
@@ -46,22 +46,22 @@
         cancel.style.display = 'block'
         index
           .search(search.value, {
-            hitsPerPage: 10,
+            hitsPerPage: 5,
           })
           .then((results) => {
             const hits = results.hits
             const data = hits.reduce((data, hit) => {
               const d = {}
               d.url = hit.url
               var section = hit.hierarchy.lvl0
-              if (hit.hierarchy.lvl6 !== null) section = section + ' [' + hit.hierarchy.lvl6 + ']'
+              if (hit.hierarchy.lvl6 !== null) section = section + '/' + hit.hierarchy.lvl6
               var breadcrumbs = Object.values(hit.hierarchy)
                 .slice(1)
                 .filter((lvl) => lvl !== null)
-                .join(' » ')
+                .join(' / ')
 
               d.breadcrumbs = ((breadcrumbs !== '') ? breadcrumbs : section)
-              d.snippet = hit._snippetResult.content.value + '...'
+              d.snippet = hit._snippetResult.content.value.split('"').join('') + '...'

Review comment:
       Yes, that makes sense, I altered the code to take care of it so it doesn't split for undefined values. Yes, we use `d.snippet` within the `summary` tag.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] aashnajena commented on pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

aashnajena commented on pull request #444:
URL: https://github.com/apache/camel-website/pull/444#issuecomment-671796447


   I think the cancel icon's position needs to be fixed for smaller screens.
   
   ![Uploading Screenshot_20200811-133410__01.jpg…]()
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] aashnajena edited a comment on pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

aashnajena edited a comment on pull request #444:
URL: https://github.com/apache/camel-website/pull/444#issuecomment-671796447


   I think the cancel icon's position needs to be fixed for smaller screens.
   
   ![Screenshot_20200811-133410__01](https://user-images.githubusercontent.com/32356795/89873289-d4a11700-dbd7-11ea-95e0-966ee298b1a5.jpg)
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] AemieJ commented on pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

AemieJ commented on pull request #444:
URL: https://github.com/apache/camel-website/pull/444#issuecomment-671799449


   @aashnajena I will look into that and fix it.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] zregvart commented on a change in pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

zregvart commented on a change in pull request #444:
URL: https://github.com/apache/camel-website/pull/444#discussion_r468865056



##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -356,7 +418,7 @@ body {
   }
 
   #search-cancel {
-    bottom: calc(50% - 0.45rem);
+    margin-top: 0.75rem;
   }
 
   #search_results {

Review comment:
       I think it looks a bit better if we take up full width on smaller screens
   ```suggestion
     #search_results {
       left: 0;
       width: 100vw;
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] zregvart merged pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

zregvart merged pull request #444:
URL: https://github.com/apache/camel-website/pull/444


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-website] zregvart commented on pull request #444: CAMEL-15335: update the search layout design

GitBox
In reply to this post by GitBox

zregvart commented on pull request #444:
URL: https://github.com/apache/camel-website/pull/444#issuecomment-672759452


   Thanks!


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]