Details
-
Improvement
-
Status: Resolved
-
Major
-
Resolution: Fixed
-
None
-
None
-
Moderate
Description
Within the small width screen, the setting is to only display the social icon button as of now. However, I thought it would be a good idea to include the search icon alongside the social button icons. On click of the social button icons, we could include animation, for instance, a fade-in effect of the search bar occupying the entire navbar. The user can do the search as required. On the left side of the search bar, there will be back arrow, which on click would give us the original navbar look with a fade-out effect.
Before I start working on this, I would like an opinion about this idea.
Attachments
Attachments
- transform-method.png
- 5 kB
- Aemie
- nav-search-icon-1220.png
- 16 kB
- Aemie
- nav-searchbar-414.png
- 3 kB
- Aemie
- nav-searchbar-1220.png
- 30 kB
- Aemie
- navbar-searchbar.png
- 2 kB
- Aemie
- navbar-search-414.png
- 10 kB
- Aemie
- issue-facing.png
- 37 kB
- Aemie
- inline-search.png
- 10 kB
- Aemie
- design-navbar.png
- 12 kB
- Aemie
- design-frontpage.png
- 63 kB
- Aemie
- bootstrap-design-frontpage.png
- 61 kB
- Aemie
- bootstrap-design-blog.png
- 52 kB
- Aemie
Issue Links
- links to
Activity
zregvart commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-652879954
Awesome @AemieJ, great effort, and thank you for tolerating my nitpickery!
----------------------------------------------------------------
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:
users@infra.apache.org
zregvart merged pull request #398:
URL: https://github.com/apache/camel-website/pull/398
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-652535935
@zregvart I simplified a bit further using your suggestion. however certain changes are necessary.
----------------------------------------------------------------
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:
users@infra.apache.org
zregvart commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r448032410
##########
File path: antora-ui-camel/src/css/blog.css
##########
@@ -223,3 +223,13 @@ article.blog p {
.blog .page-item.active a
+
+@media screen and (max-width: 1023px) {
+ article.blog:first-child {
Review comment:
This selector might be a bit problematic as it might target anything (including an element that's absolutely positioned). I'm fairly certain that to accommodate the second row on the top level navigation we need to move or add the padding/margin globaly. I think this change needs to go to `base.css` and cover all pages. We should also base this from the `-navbar-height` variable like we do for the `-body-top`. Remember these variables can change and then `5rem` is not accurate.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -387,11 +407,19 @@ body {
.navbar-fill
{ flex-grow: 1; + order: 3; +}+
+.break-row
-@media screen and (min-width: 769px) {
+@media screen and (min-width: 1024px) {
.doc > h1.page:first-child
}
+@media screen and (max-width: 1023px) {
+ .doc > h1.page:first-child {
Review comment:
Could be unnecessary if we move the `--body-top`.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -59,18 +65,26 @@ body
+@media screen and (max-width: 1277px) {
+ .navbar-brand
+}
+
.navbar-burger {
color: var(--navbar-font-color);
- background: none;
+ background: var(--navbar-background);
border: none;
outline: none;
line-height: 1; - height: var(--navbar-height);
Review comment:
Use the `--navbar-height` variable instead.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -351,6 +359,12 @@ body
+ @media screen and (min-width: 1024px) and (max-width: 1080px) {
+ .navbar-end {
+ margin-left: 1rem;
Review comment:
This is to change the left margin between the top level navigation (i.e. "Blog") and the logo? I think we can make that for all resolutions.
##########
File path: antora-ui-camel/src/css/blog.css
##########
@@ -223,3 +223,13 @@ article.blog p {
.blog .page-item.active a
+
+@media screen and (max-width: 1023px) {
+ article.blog:first-child
+
+ .blog.list aside {
+ margin-top: 4.5rem;
Review comment:
If we move the whole `--body-top` then we don't need to reposition the individual elements, like the category menu here.
##########
File path: antora-ui-camel/src/css/frontpage.css
##########
@@ -252,12 +252,17 @@ section.frontpage h2 {
}
@media screen and (max-width: 1023px) {
+ header.frontpage
+
header.frontpage h1
header.frontpage svg {
height: 10rem;
+ top: 8rem;
Review comment:
Could be unnecessary if we move the `--body-top`.
##########
File path: antora-ui-camel/src/css/frontpage.css
##########
@@ -252,12 +252,17 @@ section.frontpage h2 {
}
@media screen and (max-width: 1023px) {
+ header.frontpage
}
+@media screen and (max-width: 1023px) {
+ .static
+@media screen and (min-width: 1024px) and (max-width: 1174px) {
+ .toc.sidebar .toc-menu {
+ margin-top: 4rem;
Review comment:
Could be unnecessary if we move the `--body-top`.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -225,14 +227,14 @@ body {
}
}
-@media screen and (max-width: 479px) {
+@media screen and (max-width: 1023px) {
.navbar-menu.is-active {
display: block;
position: absolute;
height: 100vh;
box-shadow: 0 16px 16px 0 rgba(10, 10, 10, 0.1);
overflow-y: auto;
- top: 4rem;
+ top: 8rem;
Review comment:
Could be unnecessary if we move the `--body-top`.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -318,6 +320,12 @@ body
+ @media screen and (max-width: 1080px) {
+ .navbar-item
+ }
Review comment:
I'd change this in higher widths, no need for a lot of padding, 1.5rem should look okay in all resolutions.
##########
File path: antora-ui-camel/src/css/static.css
##########
@@ -1,6 +1,5 @@
.static {
margin: var(--static-margin);
- max-width: var(--static-max-width);
Review comment:
Not sure about this change, why are we making it?
##########
File path: antora-ui-camel/src/partials/header-content.hbs
##########
@@ -23,6 +23,7 @@
</div>
</div>
<div class="navbar-fill"></div>
+ <div class="break-row"></div>
Review comment:
Doesn't seem to make a difference, I've removed this from the DOM in developer tools and resized from 1023px to 300px, and the layout remained the same.
----------------------------------------------------------------
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:
users@infra.apache.org
aashnajena commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-651119961
Looks good!
----------------------------------------------------------------
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:
users@infra.apache.org
Delawen commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-650978244
That's good news!
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ edited a comment on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-650871258
@zregvart that's a good suggestion if we just want the logo to appear in the nav menu. With that, I changed breakpoint to 1024
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-650871258
@zregvart that's a good suggestion if we just want the logo to appear in the nav menu. It will space as well.
----------------------------------------------------------------
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:
users@infra.apache.org
zregvart commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-650831572
@AemieJ what if we shrunk the logo width in the nav bar to 4rem and reduced the padding between menu items from 2rem to 1.5rem, then at 1024px we can still have the big menu:
![Screenshot_2020-06-29 Home - Apache Camel](https://user-images.githubusercontent.com/1306050/85960045-af967300-b9a0-11ea-9ab3-e8232c9cd55e.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:
users@infra.apache.org
AemieJ commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-650697264
@zregvart With the current changes to the navbar menu, the breakpoint changes from 1278px to 1175px.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-650557635
@zregvart I have done highly specific changes to it from scratch solving each part. Although the breakpoint is defined right now is at 1278px but let's wait for #397 to get merged, then I could change the breakpoint for it.
It works fine as I have checked. Please check for any loopholes if you can find.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-650091358
@zregvart okay as I know what needs to be done with flex box I will try to do this from scratch. It might take a day or 2.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r446075048
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -59,18 +65,26 @@ body
+@media screen and (max-width: 1277px) {
+ .navbar-brand {
+ flex-wrap: wrap;
Review comment:
I found what's causing this issue. I will fix it in my next commit.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r446075048
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -59,18 +65,26 @@ body
+@media screen and (max-width: 1277px) {
+ .navbar-brand {
+ flex-wrap: wrap;
Review comment:
I will look into this as I didn't face this issue.
----------------------------------------------------------------
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:
users@infra.apache.org
zregvart commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-650085261
> I don't realize what part is complex yet as I now use only one input and make use of flex box and everything else is css based on the breakpoint?
The complexity I see is from adding conditionals to the navigation panel rendering the components reference and that a lot of positioning seems to be done by finding the values and putting them in, rather than deriving them from variables/using `calc`. What I see as a problem is that if we change, say the height of the navbar, this positioning needs to be recalculated again. There's also the element that was added to ensure the break on smaller screens, I'm not sure that it's needed. Using flex layout with `float`. Overall if you look at the changes here and the outcome from those it seems like a lot of changes for not that many changes in the output. I think if we were to redo this knowing the principles that are needed for this change, carefully adding/changing only the CSS that needs to be changed, we'd end up with 33%-50% of the CSS we have now.
Anyhow don't let that discourage you, this is excellent work, keep polishing this, we can use a lot of it and hopefully merge it soon.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r446074597
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -28,7 +34,7 @@ body
-@media screen and (min-width: 1024px) {
+@media screen and (min-width: 1278px) {
Review comment:
Alright, yes as the navbar menu will be changed, the breakpoint will change accordingly.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-650080705
@zregvart I will take care of it in the future PR to only include one change.
Other than that, as I have read your comments I have to make use of variables and revert the algolia search bundle to normal as well as remove class names for search cancel and search results.
I don't realize what part is complex yet as I now use only one input and make use of flex box and everything else is css based on the breakpoint?
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r446066727
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -417,18 +445,18 @@ body {
}
}
-#search-cancel {
+.search-cancel {
Review comment:
@zregvart yes, I will change it for both search cancel and search results to use it by Id directly.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r446066335
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -387,11 +407,19 @@ body {
.navbar-fill
{ flex-grow: 1; + order: 3; +}+
+.break-row {
+ display: none;
+ flex-basis: 100%;
+ height: 0;
Review comment:
The height: 0 is the reason the row will break. The display: none is only present as we don't need this change above the breakpoint.
----------------------------------------------------------------
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:
users@infra.apache.org
zregvart commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r445436538
##########
File path: antora-ui-camel/src/css/doc.css
##########
@@ -532,6 +538,19 @@
padding: 0.75rem;
}
+.doc aside
{ + float: right; + margin-left: 1rem; + width: 25rem; + margin-top: 3.5rem; +}+
+@media screen and (max-width: 1023px) {
+ .doc.side aside {
+ margin-left: -1rem;
Review comment:
That sounds like a workaround, I'd prefer that we don't implement workarounds, they tend to get back at us.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -59,18 +65,26 @@ body
+@media screen and (max-width: 1277px) {
+ .navbar-brand
+}
+
.navbar-burger {
color: var(--navbar-font-color);
- background: none;
+ background: var(--navbar-background);
border: none;
outline: none;
line-height: 1; - height: var(--navbar-height);
+ width: 2rem;
+ height: 2rem;
+ top: 1.25rem;
+ float: right;
Review comment:
Not sure if we want to use float and flex on the same element, do we need this?
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -417,18 +445,18 @@ body {
}
}
-#search-cancel {
+.search-cancel
-#search_results {
+.search-results {
Review comment:
Now we have only one element, should we target by id instead?
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -28,7 +34,7 @@ body
-@media screen and (min-width: 1024px) {
+@media screen and (min-width: 1278px) {
Review comment:
I wonder if we should wait for #397 to get merged so we don't need to change this breakpoint
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -59,18 +65,26 @@ body
+@media screen and (max-width: 1277px) {
+ .navbar-brand
+.doc aside {
+ float: right;
Review comment:
I don think this `float` is needed here
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -59,18 +65,26 @@ body
+@media screen and (max-width: 1277px) {
+ .navbar-brand
+}
+
.navbar-burger {
color: var(--navbar-font-color);
- background: none;
+ background: var(--navbar-background);
border: none;
outline: none;
line-height: 1; - height: var(--navbar-height);
Review comment:
Could we use `var(--navbar-height)` instead of `2rem` below?
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -59,18 +65,26 @@ body
+@media screen and (max-width: 1277px) {
+ .navbar-brand
+}
+
.navbar-burger {
color: var(--navbar-font-color);
- background: none;
+ background: var(--navbar-background);
Review comment:
Not sure if this is needed, doesn't seem to make a difference. Perhaps I'm missing something.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -443,19 +471,19 @@ body
-#search_results dt {
+.search-results dt
-#search_results a {
+.search-results a {
Review comment:
Now we have only one element, should we target by id instead?
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -80,20 +94,20 @@ body {
.navbar-burger span {
background: var(--navbar-font-color);
display: block;
- height: 1px;
+ height: 2px;
Review comment:
Thinner looked more elegant to me, not sure if we need to make it thicker.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -417,18 +445,18 @@ body {
}
}
-#search-cancel {
+.search-cancel {
Review comment:
Now we have only one element, should we target by id instead?
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -156,14 +172,19 @@ body
-@media screen and (max-width: 1023px) and (min-width: 480px) {
+@media screen and (max-width: 1277px) {
+ .navbar-menu.is-active {
+ top: 7.75rem;
Review comment:
I think we should derive this from the navbar height, I can see that this will go out of alignment if we change the navbar height variable. Perhaps using the variable here or calculating the hight would be better.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -443,19 +471,19 @@ body
-#search_results dt {
+.search-results dt
}
+@media screen and (max-width: 1023px) {
+ .nav-container {
+ top: 7.75rem;
Review comment:
Perhaps derive a value using `calc` or use a variable here.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -475,7 +503,7 @@ body
-.results-hidden #search_results {
+.results-hidden .search-results {
Review comment:
Now we have only one element, should we target by id instead?
##########
File path: antora-ui-camel/src/css/nav.css
##########
@@ -44,10 +50,25 @@
@media screen and (min-width: 1024px) {
.nav
+
+ .nav-category
+
+ .nav-components {
+ top: 6.75rem;
Review comment:
Perhaps derive a value using calc or use a variable here.
##########
File path: antora-ui-camel/src/css/nav.css
##########
@@ 118,6 +152,12 @@ html.is-clipped-nav
+@media screen and (max-width: 1277px) {
+ .nav-menu {
+ top: 3.5rem;
Review comment:
Perhaps derive a value using calc or use a variable here.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -387,11 +407,19 @@ body {
.navbar-fill
{ flex-grow: 1; + order: 3; +}+
+.break-row {
+ display: none;
+ flex-basis: 100%;
+ height: 0;
Review comment:
Do we need `height: 0` with `display: none`?
##########
File path: antora-ui-camel/src/css/nav.css
##########
@@ -44,10 +50,25 @@
@media screen and (min-width: 1024px) {
.nav
+
+ .nav-category {
+ top: 8.5rem;
Review comment:
Perhaps derive a value using calc or use a variable here.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -443,19 +471,19 @@ body
-#search_results dt {
+.search-results dt
-#search_results a {
+.search-results a
-#search_results a:hover {
+.search-results a:hover {
Review comment:
Now we have only one element, should we target by id instead?
##########
File path: antora-ui-camel/src/partials/header-content.hbs
##########
@@ -6,32 +6,39 @@
<div class="navbar-end">
#withMenuData
#each @items
- #if children
- <div class="navbar-item has-dropdown is-hoverable">
+ #if children
+ <div class="navbar-item has-dropdown is-hoverable">
<a class="navbar-link navbar-topics" href="#">name</a>
<div class="navbar-dropdown">
#each children
<a class="navbar-item" href="url">name</a>
/each
</div> - </div>
- else
- <a class="navbar-item navbar-topics" href="url">name</a>
- /if
+ </div>
+ else
+ <a class="navbar-item navbar-topics" href="url">name</a>
+ /if
/each
/withMenuData
</div>
</div>
<div class="navbar-fill"></div>
+ <div class="break-row"></div>
<div class="navbar-search results-hidden">
<input id="search" class="search" placeholder="Search" autocomplete="off"> - <img src= "/_/img/cancel.svg" alt="Clear" id="search-cancel">
- <div id="search_results"></div>
+ <img src= "/_/img/cancel.svg" alt="Clear" id="search-cancel" class="search-cancel">
+ <div id="search_results" class="search-results"></div>
Review comment:
Do we need the `search-results` class here?
##########
File path: antora-ui-camel/src/js/vendor/algoliasearch.bundle.js
##########
@@ -6,10 +6,12 @@
window.addEventListener('load', () => {
const client = algoliasearch('BH4D9OD16A', '16e3a9155a136e4962dc4c206f8278bd')
const index = client.initIndex('apache_camel')
- const search = document.querySelector('#search')
+ const search = document.querySelector('.search')
const container = search.parentNode - const results = document.querySelector('#search_results')
- const cancel = document.querySelector('#search-cancel')
+ const results = document.querySelector('.search-results')
+ const cancel = document.querySelector('.search-cancel')
+ const className = 'navbar-search'
+ const hiddenClass = 'navbar-search results-hidden'
Review comment:
Do we still need these changes to the JS code now that we have only one search input?
##########
File path: antora-ui-camel/src/partials/header-content.hbs
##########
@@ -6,32 +6,39 @@
<div class="navbar-end">
#withMenuData
#each @items
- #if children
- <div class="navbar-item has-dropdown is-hoverable">
+ #if children
+ <div class="navbar-item has-dropdown is-hoverable">
<a class="navbar-link navbar-topics" href="#">name</a>
<div class="navbar-dropdown">
#each children
<a class="navbar-item" href="url">name</a>
/each
</div> - </div>
- else
- <a class="navbar-item navbar-topics" href="url">name</a>
- /if
+ </div>
+ else
+ <a class="navbar-item navbar-topics" href="url">name</a>
+ /if
/each
/withMenuData
</div>
</div>
<div class="navbar-fill"></div>
+ <div class="break-row"></div>
<div class="navbar-search results-hidden">
<input id="search" class="search" placeholder="Search" autocomplete="off"> - <img src= "/_/img/cancel.svg" alt="Clear" id="search-cancel">
- <div id="search_results"></div>
+ <img src= "/_/img/cancel.svg" alt="Clear" id="search-cancel" class="search-cancel">
Review comment:
Do we need the `search-cancel` class here?
##########
File path: antora-ui-camel/src/partials/nav.hbs
##########
@@ -1,5 +1,9 @@
<div class="nav-container"#if page.component data-component="page.component.name" data-version="page.version"/if>
+ #if (eq page.component.name 'components')
+ <aside class="nav nav-components">
+ else
<aside class="nav">
+ /if
Review comment:
I think this could be simpler, all `aside.nav` should cater to the same positioning, not sure why components would be different, perhaps the parent of this aside (`.nav-container`) should be positioned instead.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-647144105
@zregvart I worked with the flexbox method and applied it to the website, it works out neatly. Please take a look at it and do let me know if anything further is required to be done.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ edited a comment on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-647089895
@zregvart I found a method using flex-box by creating [this jsfiddle](https://jsfiddle.net/kh0n9yge/), please check if you find any loophole. Else, I will move forward on implementing this on the site.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-647089895
@zregvart I found a method using flex-box on [this jsfiddle](https://jsfiddle.net/kh0n9yge/), please check if you find any loophole. Else, I will move forward on implementing this on the site.
----------------------------------------------------------------
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:
users@infra.apache.org
zregvart commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-647041656
@AemieJ I don't think CSS transform should be used for responsive design. I think responsive design should be based on flexbox. Can you research the example I provided and see if it can be applied to the website?
----------------------------------------------------------------
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:
users@infra.apache.org
zregvart delawen aashnajena There has been a discussion on PR to not make use of two separate divs to achieve the search design. zregvart suggested the idea of using order as CSS property to use, however, there was a loophole in it as discussed in the PR but was clever so I thought of using a single CSS property to achieve the same design and I landed up with the translate property of CSS, there are some permanent values to tweak.
In this, I just tweaked the translate values to achieve this. I am aware the logo isn't visible but the reason is the logo is used as a background image so it's easily overlayed. I was thinking of using them img within the a tag for it to be visible but I plan to make changes if you're comfortable with the use of transform property of CSS alone. What are your opinions on it?
AemieJ edited a comment on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-646649037
@zregvart Another method if we don't want to make use of two search divs, we could increase the height of the navbar and use the translate property of CSS to control it. What do you think about that?
![transform-method](https://user-images.githubusercontent.com/44139348/85195030-d4c61a00-b2ec-11ea-8508-34894a46ee04.png)
In this, I just tweaked the translate values to achieve this. I am aware the logo isn't visible but the reason is the logo is used as background image so it's easily overlayed. I was thinking of using the `img` within the `a` tag for it to be visible but I plan to make changes if you're comfortable with the use of transform property of CSS alone.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-646649037
@zregvart Another method if we don't want to make use of two search divs, we could increase the height of the navbar and use the translate property of CSS to control it. What do you think about that?
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-646442106
> @AemieJ I've created [this JSFiddle](https://jsfiddle.net/ry2Lhwtd/) to illustrate how this can be done with CSS only. The key is that in flex layout you can specify the order of the elements and that a break can be forced after an element pushing the search div to the second row.
I tried using the method you described in jsfiddle. However, in the output, I observed that at a point it will only push down when there is no more space to be accommodated in the main navbar. I observed at a point where only the navbar is pushed down and the search bar is after the social icon due to ordering, I don't think we want that kind of behavior.
![behavior-flex](https://user-images.githubusercontent.com/44139348/85099315-b98ada00-b21a-11ea-9eb9-a756563b9ea3.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:
users@infra.apache.org
zregvart commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-646246638
@AemieJ I've created [this JSFiddle](https://jsfiddle.net/ry2Lhwtd/) to illustrate how this can be done with CSS only. The key is that in flex layout you can specify order of the elements, and that a break can be forced after an element pushing the search div to second row.
----------------------------------------------------------------
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:
users@infra.apache.org
zregvart commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r442419344
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -394,6 +399,18 @@ body
+.search-bar {
+ top: 4.1rem;
+ position: sticky;
+ position: -webkit-sticky;
Review comment:
Should be the same as `sticky`, it can be used unprefixed according to [caniuse.com](https://caniuse.com/#feat=css-sticky) – 91.35% browsers don't require the prefix.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r442406788
##########
File path: antora-ui-camel/src/js/vendor/algoliasearch.bundle.js
##########
@@ -25,17 +28,23 @@
}
}
+ search.addEventListener('click', function (e)
{ + e.stopPropagation() + })
+
search.addEventListener(
'keyup',
debounce((key) => {
+ if (condition) document.querySelector('.navbar-burger').style.marginTop = '1rem'
Review comment:
Yes, I experimented and it is possible through HTML and CSS only. I will push the changes for the same.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r442297448
##########
File path: antora-ui-camel/src/css/vars.css
##########
@@ -131,11 +131,14 @@
--footer-height: 23rem; /* empirically corresponding current design of the foooter */
/* dimensions */
-navbar-height: calc(73 / var(-rem-base) * 1rem);
+ -toolbar-nav-height: calc(139.5 / var(-rem-base) * 1rem);
+ -panel-height: calc(54 / var(-rem-base) * 1rem);
Review comment:
Alright, the toolbar nav height is introduced for the width < 1275 as we introduce the whole search design below the actual navbar including the logo and social icons.
Now with the changes, I observed that within the menu panel height under the docs section, the panel items overflow to the footer due to the height defined. Hence, the panel height is used to prevent the overflow.
##########
File path: antora-ui-camel/src/css/vars.css
##########
@@ -131,11 +131,14 @@
--footer-height: 23rem; /* empirically corresponding current design of the foooter */
/* dimensions */
-navbar-height: calc(73 / var(-rem-base) * 1rem);
+ -toolbar-nav-height: calc(139.5 / var(-rem-base) * 1rem);
+ -panel-height: calc(54 / var(-rem-base) * 1rem);
Review comment:
Alright, the toolbar nav height is introduced for the width < 1275 as we introduce the whole search design below the actual navbar including the logo and social icons.
Now with the changes, I observed that within the menu panel height under the docs section, the panel items overflow to the footer due to the height defined. Hence, the panel height is used to prevent the overflow.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r442277992
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -394,6 +399,18 @@ body
+.search-bar
{ + top: 4.1rem; + position: sticky; + position: -webkit-sticky; + z-index: 4; +}+
+.navbar-small {
+ display: none;
+ background: #fff;
Review comment:
It is being displayed for width < 1275px.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r442268862
##########
File path: antora-ui-camel/src/css/blog.css
##########
@@ -25,6 +25,16 @@ article.blog:first-child
+@media screen and (max-width: 1023px) {
+ .blog.list aside
+
+ article.blog-post:first-child {
Review comment:
No, this change corresponds particularly to the blog post only.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r441968840
##########
File path: antora-ui-camel/src/js/vendor/algoliasearch.bundle.js
##########
@@ -6,10 +6,13 @@
window.addEventListener('load', () => {
const client = algoliasearch('BH4D9OD16A', '16e3a9155a136e4962dc4c206f8278bd')
const index = client.initIndex('apache_camel')
- const search = document.querySelector('#search')
+ const condition = window.innerWidth < 1275
+ const search = condition ? document.querySelector('.search-small') : document.querySelector('.search')
Review comment:
I don't know why it seems complex 'cause the ternary operator does make the code readable. And the reason that two searches are included is just for breakpoint differentiation is 'cause if we were to include the single search input that is present within the nav, it will be more complex to position it the way we want in the design ( I will give it a try though ). Also, in my opinion, the use of two different div for the search design for two breakpoints seems much readable and it's easier to differentiate them using CSS as well.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r441960661
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -394,6 +399,18 @@ body
+.search-bar {
+ top: 4.1rem;
+ position: sticky;
+ position: -webkit-sticky;
Review comment:
Yes, we need this 'cause chrome tends to behave in a weird manner at times compared to the other browsers such as firefox and safari. Hence, both the position is required.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r441959603
##########
File path: antora-ui-camel/src/css/doc.css
##########
@@ -532,6 +538,19 @@
padding: 0.75rem;
}
+.doc aside
{ + float: right; + margin-left: 1rem; + width: 25rem; + margin-top: 3.5rem; +}+
+@media screen and (max-width: 1023px) {
+ .doc.side aside {
+ margin-left: -1rem;
Review comment:
This is for the category issue that I raised. The box has been shifted towards the right, hence this just shifts it back to original position to the left.
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r441959286
##########
File path: antora-ui-camel/src/css/doc.css
##########
@@ -37,6 +37,12 @@
margin: 1.5rem 0;
}
+@media screen and (max-width: 1274px) {
+ .static > h1:first-child {
+ margin: 4.05556rem 0 1.5rem 0;
Review comment:
Alright, I believe those values are already calculated within the `vars.css`, I will directly make use of them.
----------------------------------------------------------------
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:
users@infra.apache.org
zregvart commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r441865179
##########
File path: antora-ui-camel/src/css/vars.css
##########
@@ -131,11 +131,14 @@
--footer-height: 23rem; /* empirically corresponding current design of the foooter */
/* dimensions */
-navbar-height: calc(73 / var(-rem-base) * 1rem);
+ -toolbar-nav-height: calc(139.5 / var(-rem-base) * 1rem);
+ -panel-height: calc(54 / var(-rem-base) * 1rem);
-toolbar-height: calc(45 / var(-rem-base) * 1rem);
-drawer-height: var(-toolbar-height);
-body-top: calc(var(-navbar-height) + 1rem);
-body-min-height: calc(100vh - var(-navbar-height));
-nav-height: calc(var(body-min-height) - var(-toolbar-height));
+ -nav-menu-panel-height: calc(100vh - var(-panel-height));
-nav-heightdesktop: var(-body-min-height);
-nav-panel-height: calc(var(nav-height) - var(-drawer-height));
Review comment:
This value needs to be adjusted, the height of the search bar needs to be subtracted from the height to fit the explore drawer
----------------------------------------------------------------
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:
users@infra.apache.org
zregvart commented on a change in pull request #398:
URL: https://github.com/apache/camel-website/pull/398#discussion_r441848880
##########
File path: antora-ui-camel/gulp.d/tasks/build.js
##########
@@ -99,8 +99,8 @@ module.exports = (src, dest, preview) => () => {
)
.pipe(buffer())
.pipe(terser())
- // NOTE use this statement to bundle a JavaScript library that cannot be browserified, like jQuery
- //vfs.src(require.resolve('<package-name-or-require-path>'), opts).pipe(concat('js/vendor/<library-name>.js')),
+ // NOTE use this statement to bundle a JavaScript library that cannot be browserified, like jQuery
+ //vfs.src(require.resolve('<package-name-or-require-path>'), opts).pipe(concat('js/vendor/<library-name>.js')),
Review comment:
Can you move this to a separate commit/pull request.
##########
File path: antora-ui-camel/src/css/base.css
##########
@@ -62,11 +62,13 @@ th
-em em { /* stylelint-disable-line */
+em em {
+ /* stylelint-disable-line */
Review comment:
Can you move this to a separate commit/pull request.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -394,6 +399,18 @@ body
+.search-bar {
+ top: 4.1rem;
+ position: sticky;
+ position: -webkit-sticky;
Review comment:
Do we need this?
```suggestion
```
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -406,6 +423,28 @@ body
+.navbar-small input
{ + width: 93vw; +}+
+@media screen and (max-width: 1024px) {
+ .navbar-small input
+}
+
+@media screen and (min-width: 425px) and (max-width: 575px) {
+ .navbar-small input
+}
+
+@media screen and (max-width: 425px) {
+ .navbar-small input
+}
+
.navbar-search input:focus-within
-.toc .toc-menu li[data-level="2"] a {
+.toc .toc-menu li[data-level='2'] a
-.toc .toc-menu li[data-level="3"] a {
+.toc .toc-menu li[data-level='3'] a {
Review comment:
Style changes in this file should be on a separate commit or in a new pull request.
##########
File path: antora-ui-camel/src/js/vendor/algoliasearch.bundle.js
##########
@@ -6,10 +6,13 @@
window.addEventListener('load', () => {
const client = algoliasearch('BH4D9OD16A', '16e3a9155a136e4962dc4c206f8278bd')
const index = client.initIndex('apache_camel')
- const search = document.querySelector('#search')
+ const condition = window.innerWidth < 1275
+ const search = condition ? document.querySelector('.search-small') : document.querySelector('.search')
Review comment:
I'd prefer if we had a CSS only solution for this, not sure if there's need for two inputs for search. This way we make the implementation much more complex.
##########
File path: antora-ui-camel/src/css/blog.css
##########
@@ -25,6 +25,16 @@ article.blog:first-child
+@media screen and (max-width: 1023px) {
+ .blog.list aside
+
+ article.blog-post:first-child
+.doc aside
{ + float: right; + margin-left: 1rem; + width: 25rem; + margin-top: 3.5rem; +}+
+@media screen and (max-width: 1023px) {
+ .doc.side aside {
+ margin-left: -1rem;
Review comment:
Not sure about negative margins, seems hacky.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -61,15 +61,15 @@ body {
.navbar-burger
{ color: var(--navbar-font-color); - background: none; + background: #fff; Review comment: ```suggestion background: var(--navbar-background); ``` ########## File path: antora-ui-camel/src/css/doc.css ########## @@ -37,6 +37,12 @@ margin: 1.5rem 0; }+@media screen and (max-width: 1274px) {
+ .static > h1:first-child
.doc b.button::before {
Review comment:
Perhaps put the codestyle changes in a separate commit/pull request.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -406,6 +423,28 @@ body
+.navbar-small input
{ + width: 93vw; +}+
+@media screen and (max-width: 1024px) {
+ .navbar-small input
+}
+
+@media screen and (min-width: 425px) and (max-width: 575px) {
+ .navbar-small input
+}
+
+@media screen and (max-width: 425px) {
+ .navbar-small input
+}
Review comment:
I'd either use percentages or rem.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -426,9 +465,9 @@ body
-#search_results {
+.search-results {
top: 3.38rem;
- background: #fffe;
+ background: #fff;
Review comment:
Use variables
##########
File path: antora-ui-camel/src/css/site.css
##########
@@ -22,4 +22,4 @@
@import 'release.css';
@import 'misc.css';
@import 'community.css';
-@import 'projects.css'
+@import 'projects.css';
Review comment:
Style changes in this file should be on a separate commit or in a new pull request.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -394,6 +399,18 @@ body
+.search-bar
{ + top: 4.1rem; + position: sticky; + position: -webkit-sticky; + z-index: 4; +}+
+.navbar-small {
+ display: none;
+ background: #fff;
Review comment:
Does the element need a background if it's not displayed?
```suggestion
background: #fff;
```
##########
File path: antora-ui-camel/src/css/pagination.css
##########
@@ -29,11 +29,11 @@ nav.pagination span::before {
}
nav.pagination .prev::before
{ Review comment: Style changes in this file should be on a separate commit or in a new pull request. ########## File path: antora-ui-camel/src/js/vendor/algoliasearch.bundle.js ########## @@ -25,17 +28,23 @@ }}
+ search.addEventListener('click', function (e)
{ + e.stopPropagation() + })
+
search.addEventListener(
'keyup',
debounce((key) => {
+ if (condition) document.querySelector('.navbar-burger').style.marginTop = '1rem'
Review comment:
I'm not sure that the content security policy will allow changing the style via JavaScript, anyhow we need to minimize the JavaScript and focus on style changes via CSS only.
##########
File path: antora-ui-camel/src/css/vars.css
##########
@@ -131,11 +131,14 @@
--footer-height: 23rem; /* empirically corresponding current design of the foooter */
/* dimensions */
-navbar-height: calc(73 / var(-rem-base) * 1rem);
+ -toolbar-nav-height: calc(139.5 / var(-rem-base) * 1rem);
+ -panel-height: calc(54 / var(-rem-base) * 1rem);
Review comment:
These are some odd values, how did we came to them?
##########
File path: antora-ui-camel/src/partials/header-content.hbs
##########
@@ -5,33 +5,46 @@
<div id="topbar-nav" class="navbar-menu">
<div class="navbar-end">
#withMenuData
- #each @items
Review comment:
Not sure about this, the code was indented to make it more readable.
----------------------------------------------------------------
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:
users@infra.apache.org
aashnajena removed a comment on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-645614676
Hey, how are the search results appearing for <1275px? In the preview, I can see search results appearing only for >1275px screens
----------------------------------------------------------------
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:
users@infra.apache.org
aashnajena commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-645614676
Hey, how are the search results appearing for <1275px? In the preview, I can see search results appearing only for >1275px screens
----------------------------------------------------------------
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:
users@infra.apache.org
AemieJ opened a new pull request #398:
URL: https://github.com/apache/camel-website/pull/398
-
- DESCRIPTION ON THE PROBLEM
Below the width of 1275px for the screen, the search panel couldn't be included. This is very inconvenient as we require the presence of search for all screen width. In addition to this problem, I observed another with the toolbar. When clicked on the nav toggle within the toolbar, the box with the list was shifted to the right and it seemed like a bug present on the website.
-
-
- BEFORE TOOLBAR PANEL DESIGN
![old-toolbar](https://user-images.githubusercontent.com/44139348/84804375-4e75b380-b020-11ea-81d6-30ffbd68ca0e.png)
- BEFORE TOOLBAR PANEL DESIGN
-
-
-
- AFTER TOOLBAR PANEL DESIGN
![new-toolbar-design](https://user-images.githubusercontent.com/44139348/84804388-52093a80-b020-11ea-9248-66ba2c669c5b.png)
- AFTER TOOLBAR PANEL DESIGN
-
-
- DESIGN ALTERNATIVE PROVIDED
The search design has been inspired by the design of https://getbootstrap.com/docs/4.5/getting-started/introduction/.
The search bar design is present for the width < 1275 where 1275 is the breakpoint. Here, the search bar is present alongside the navbar burger right below the major navbar. Due to the addition of components, changes are made to the nav-panel on various pages and docs as well.
The hierarchy of this design is as follows:
```
– Search Results # If the navbar menu and/or nav panel item are open, the search results will be shown above them
– Navbar Menu # If the nav panel item is open, the navbar menu will appear on top of it
– Nav Panel Items # Least in the hierarchy
```
-
-
- BASIC DESIGN IMPLEMENTATION EXAMPLE
![search-bar-design](https://user-images.githubusercontent.com/44139348/84804532-8f6dc800-b020-11ea-9504-b0552a2b210b.png)
- BASIC DESIGN IMPLEMENTATION EXAMPLE
-
----------------------------------------------------------------
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:
users@infra.apache.org
Aemie it'll be easier to help with this when we see the code, so I'll wait for the pull request. One note though, we can't deviate from Antora UI too much, that would make it difficult to rebase on newer versions. That's the reason why make changes via variables and create separate CSS files with class prefixes to avoid collision.
zregvart aashnajena delawen The thing is I have implemented it but obviously, as there is an additional component, I had to make changes to the different areas as well especially nav panel menu. I have resolved any additional issues that raised except one. The only issue I am left with is the explore menu panel under documentation which isn't fixed at the bottom of the screen as it is by default.
delawen I have resolved the issue for that. I will be creating a PR after I am done with complete implementation.
Do you have the netlify link for checking the tested website? Maybe those uris work on a separated set of stylesheets?
zregvart I had a query, I have implemented the design and it works, however it isn't visible in any of the user-manual, components and the individual project pages, is it 'cause the website needs to be built to see the change? And I have based it on 2 breakpoints the common being 1275 as I explained before, so if you're okay with that I will push the code in a day or 2.
delawen yes in the general browser, 1024 is the break-point however in the camel website due to inclusion of a logo, navbar-menu, search and social icons, everything fits perfectly till 1275px itself. At 1024, there is no space for including the search in the navbar at all. That's why I chose 1275 as the break-point in this case.
Yes, I think we should be consistent and have only two breakpoints. Keep it simple
Maybe 1024 is a more standard size than 1275? Check https://www.w3schools.com/browsers/browsers_display.asp
zregvart delawen I wanted to ask regarding the breakpoints, currently there are three breakpoints, < 1024, 1024 - 1275, >=1275 and I think we need to be consistent with the website and keep only 2 breakpoints one with the navbar burger with the design as show and one with the default design tho with bigger icons. So how do you want to approach this, currently, for width >= 1275, the default design fits perfectly. So I thought it's best to keep the breakpoint at 1275 where the design I showed would be for < 1275 width size. What are your opinions?
zregvart no I am not asking that. What I am asking is when the side menu panel is open and you open the navbar menu, the navbar menu is overlayed on top of the menu panel. In that way, if the navbar is open and I try to search, the search results should overlay the navbar right?
So I am asking whether it is the best choice to make the z-index of search greater than that of the navbar menu for the small width screen?
Aemie I’m sorry, I don’t quite understand the question. Are you referring to the categories filter present on blog section? I don’t think that should be part of the menu.
zregvart I had a query regarding the hierarchy structure, there is a toolbar in blogs and other pages. So for those, we know navbar burger with its menu has higher hierarchy so for search results, I thought it would be better to have a greater hierarchy than the navbar-menu, does that sound good to you?
zregvart delawen aashnajena, as I really like the use of a search bar in the bootstrap website suggested by Zoran so I thought of modifying it but the design really looks good on all the pages, I think we could go ahead with implementing it, what are your opinions?
FRONT PAGE DESIGN
DOCS PAGES DESIGN
I don't like the design of search implemented with typescript much but that of bootstrap is really good. Docsearch and my implementation is similar. Also the only problem I am trying to state here is your idea of design and that of bootstrap looks amazing with screen width < 1024px when the nav burger appears. From width ranging from 1024 - 1220 there is no navbar burger and no search bar as well. And using bootstrap design to cover the entire width right below navbar might not look neat. I will implement the idea of bootstrap and see on this range of width and let you know.
Aemie I guess one option would be to place the search at the bottom of the mobile menu, similar to how https://www.typescriptlang.org/ does it. I was looking at how projects listed on https://docsearch.algolia.com/ approach this, and seems like a fair number of them opt to show the search input always and in almost full width. I think the approach on https://getbootstrap.com/docs/ is particularly good one.
zregvart if we were to include the design method you suggested, then we need to think about how we are going to be presenting the search results as well and this is applicable for width < 1024px only however for width between 1024 <= width <= 1220px, we need to include search too and the method I suggested would be more applicable at that point. What do you think?
Yea that can be done, I just wanted to implement the design concept but yea that change even I thought was necessary to make.
aashnajena How does this look to you?
This seems good to me. I too think that the search icon should be accessible without having to open the menu.
I have one suggestion. In this image, the two close buttons are a little confusing. I understand that one is for clearing the search and other for closing the search bar. Can we use a bigger cross icon, identical to the one used for closing the main menu? It's a bigger touch target, plus since it's used for closing the menu, perhaps it would help differentiate between the "clear" and "close" functionalities.
zregvart aashnajena I have added one of the alternative designs I created using js script. I will try to implement the method zregvart suggested but I find the use of this alternative better for a docs site.
aashnajena Aashna, yes I do feel it will look quite clustered if put below the navbar and about the screen result width could be adjusted to the width of the search bar itself.
zregvart Zoran, I have seen that PR, my only concern with that design is its accessibility. Also, if you take a look at the various websites including myntra, Amazon, the searn icon is present in the navbar itself as it can be easily accessed by the user on click. And if you put search embedded with the menu for smaller screen width, when you make a search, due to the light opacity of the search result box, it will make it look messy.
Looks good. We do need to be careful about the size of the touch targets, making sure that users don't trigger other links by mistake, i.e. having enough padding around the individual icons. I also like the search being embedded in the menu item, some work has been done on this in PR#255, good application of this is in the github.com responsive design.
I agree that we should have the search option on smaller screens. Aemie this would also mean that we have to adjust the search result styling for smaller screens. Another thought : If we open up a search bar below the header instead of overwriting it, would that look too cluttered?
zregvart, aashnajena what are your views on this? The state will toggle between 1 & 2 on click.
AemieJ commented on pull request #398:
URL: https://github.com/apache/camel-website/pull/398#issuecomment-653111799
@zregvart well it certainly helped with simplifying the code as much as possible, so I am glad and 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:
users@infra.apache.org