You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2020/08/04 08:54:18 UTC
[GitHub] [camel-website] AemieJ opened a new pull request #448: Improvize on the style of navbar and navbar items layout
AemieJ opened a new pull request #448:
URL: https://github.com/apache/camel-website/pull/448
This PR deals with the following changes in the layout:
1. For the Desktop ( > = 1024px)
* Remove the border-bottom and apply a mild box-shadow[1]
* Replace the color change on hover to the navbar items with a border-bottom on hover to the navbar items.
* Increase the size of navbar tools such that the navbar looks consistent.
> [1] the box-shadow appears only when scrolled through the webpage.
2. For the smaller screen width ( < 1024px)
* In here, re-structured the navbar cross button.
* Based on designs suggested on the [Jira Issue](https://issues.apache.org/jira/browse/CAMEL-15140?jql=project%20%3D%20CAMEL%20AND%20component%20%3D%20website), this [mockup](https://issues.apache.org/jira/secure/attachment/13007475/Mockup-7.png) has been implemented.
3. Extra Changes
* Remove the about dropdown from the navbar
* Remove any link related to the dropdown within the navbar.
----------------------------------------------------------------
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
[GitHub] [camel-website] AemieJ commented on pull request #448: Improvize on the style of navbar and navbar items layout
Posted by GitBox <gi...@apache.org>.
AemieJ commented on pull request #448:
URL: https://github.com/apache/camel-website/pull/448#issuecomment-669687737
@zregvart I have performed all the required changes in the latest version.
----------------------------------------------------------------
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
[GitHub] [camel-website] zregvart commented on pull request #448: Improvize on the style of navbar and navbar items layout
Posted by GitBox <gi...@apache.org>.
zregvart commented on pull request #448:
URL: https://github.com/apache/camel-website/pull/448#issuecomment-672758635
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
[GitHub] [camel-website] AemieJ commented on pull request #448: Improvize on the style of navbar and navbar items layout
Posted by GitBox <gi...@apache.org>.
AemieJ commented on pull request #448:
URL: https://github.com/apache/camel-website/pull/448#issuecomment-671934176
@zregvart I have rebased it and use the `config.toml` to include the images for the navbar.
----------------------------------------------------------------
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
[GitHub] [camel-website] zregvart commented on a change in pull request #448: Improvize on the style of navbar and navbar items layout
Posted by GitBox <gi...@apache.org>.
zregvart commented on a change in pull request #448:
URL: https://github.com/apache/camel-website/pull/448#discussion_r465337612
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -35,26 +37,27 @@ body {
}
@media screen and (min-width: 1024px) {
- .navbar-end > .navbar-item,
- .navbar-end .navbar-link {
+ .navbar-end > .navbar-item-section .navbar-item {
color: var(--navbar-font-color);
+ cursor: pointer;
}
- .navbar-end > a.navbar-item:hover,
- .navbar-end .navbar-link:hover {
- color: var(--color-asf-dark-blue);
- }
-
- .navbar-end .navbar-link::after {
- border-color: var(--navbar-font-color);
+ .navbar-end > .navbar-item-section .navbar-item::after {
+ transition: all ease-in-out 0.2s;
+ content: '';
+ position: absolute;
+ height: 2px;
+ width: 0;
+ top: var(--navbar-height);
Review comment:
This is top is positioned relative to [the ancestor](https://developer.mozilla.org/en-US/docs/Web/CSS/position#absolute), not top of the page, so having a `top: var(--navbar-height);` here makes it a bit odd:
I'd remove it, looks better without it:
```suggestion
```
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -35,26 +37,27 @@ body {
}
@media screen and (min-width: 1024px) {
- .navbar-end > .navbar-item,
- .navbar-end .navbar-link {
+ .navbar-end > .navbar-item-section .navbar-item {
color: var(--navbar-font-color);
+ cursor: pointer;
Review comment:
This targets anchor tags, so the default should be `cursor: pointer`, am I missing something?
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -174,30 +161,50 @@ body {
top: 8rem;
left: 0;
right: 0;
- padding: 1rem 2rem;
+ padding: 3rem 1rem 1rem 1.5rem;
}
- .has-dropdown:not(.is-active) > .navbar-dropdown {
- display: none;
+ .navbar-item-section {
+ width: 47%;
+ height: 10rem;
+ background: var(--color-camel-orange);
+ border-radius: 15px;
+ margin: 0 0.5rem 1.5rem 0;
}
- .has-dropdown:not(.is-active) > .navbar-link::after {
- transform: rotate(-45deg);
+ .navbar-item-section img {
+ display: block;
+ margin-left: calc(50% - 2.5rem);
+ margin-top: 1.5rem;
}
- .has-dropdown.is-active > .navbar-dropdown {
- display: block;
- position: relative;
- border: none;
+ .navbar-item.navbar-topics {
+ text-align: center;
+ color: var(--navbar-background);
+ text-transform: uppercase;
+ font-size: 20px;
}
- .has-dropdown.is-active > .navbar-link::after {
- transform: rotate(135deg);
+ @media screen and (min-width: 480px) and (max-width: 626px) {
+ .navbar-item.navbar-topics {
+ font-size: 18px;
+ }
}
- .navbar-item.navbar-topics,
- .navbar-link.navbar-topics {
- color: var(--navbar-font-color);
+ @media screen and (max-width: 480px) {
+ .navbar-item.navbar-topics {
+ font-size: 15px;
+ }
+ }
+
+ @media screen and (max-width: 375px) {
+ .navbar-item-section {
+ width: 45%;
+ }
+
+ .navbar-item.navbar-topics {
+ font-size: 14px;
Review comment:
We should do `rem`s for `font-size`.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -174,30 +161,50 @@ body {
top: 8rem;
left: 0;
right: 0;
- padding: 1rem 2rem;
+ padding: 3rem 1rem 1rem 1.5rem;
}
- .has-dropdown:not(.is-active) > .navbar-dropdown {
- display: none;
+ .navbar-item-section {
+ width: 47%;
+ height: 10rem;
+ background: var(--color-camel-orange);
+ border-radius: 15px;
+ margin: 0 0.5rem 1.5rem 0;
}
- .has-dropdown:not(.is-active) > .navbar-link::after {
- transform: rotate(-45deg);
+ .navbar-item-section img {
+ display: block;
+ margin-left: calc(50% - 2.5rem);
+ margin-top: 1.5rem;
}
- .has-dropdown.is-active > .navbar-dropdown {
- display: block;
- position: relative;
- border: none;
+ .navbar-item.navbar-topics {
+ text-align: center;
+ color: var(--navbar-background);
+ text-transform: uppercase;
+ font-size: 20px;
}
- .has-dropdown.is-active > .navbar-link::after {
- transform: rotate(135deg);
+ @media screen and (min-width: 480px) and (max-width: 626px) {
+ .navbar-item.navbar-topics {
+ font-size: 18px;
Review comment:
We should do `rem`s for `font-size`.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -174,30 +161,50 @@ body {
top: 8rem;
left: 0;
right: 0;
- padding: 1rem 2rem;
+ padding: 3rem 1rem 1rem 1.5rem;
Review comment:
I think this whole `.navbar-divider` rule can be removed, I can't find any elements with `class="navbar-divider"` in our markup.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -63,6 +66,7 @@ body {
display: flex;
flex-grow: 1;
height: var(--navbar-height);
+ outline: 0;
Review comment:
This is targeting a `div` which doesn't have an outline, so it's not needed. Even if it were targeting an element that would be focusable/actionable, we would not want to remove the outline for folk that use keyboard to navigate and see the element that they navigated to via the keyboard.
```suggestion
```
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -174,30 +161,50 @@ body {
top: 8rem;
left: 0;
right: 0;
- padding: 1rem 2rem;
+ padding: 3rem 1rem 1rem 1.5rem;
}
- .has-dropdown:not(.is-active) > .navbar-dropdown {
- display: none;
+ .navbar-item-section {
+ width: 47%;
+ height: 10rem;
+ background: var(--color-camel-orange);
+ border-radius: 15px;
+ margin: 0 0.5rem 1.5rem 0;
}
- .has-dropdown:not(.is-active) > .navbar-link::after {
- transform: rotate(-45deg);
+ .navbar-item-section img {
+ display: block;
+ margin-left: calc(50% - 2.5rem);
+ margin-top: 1.5rem;
}
- .has-dropdown.is-active > .navbar-dropdown {
- display: block;
- position: relative;
- border: none;
+ .navbar-item.navbar-topics {
+ text-align: center;
+ color: var(--navbar-background);
+ text-transform: uppercase;
+ font-size: 20px;
Review comment:
We should do `rem`s for `font-size`.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -174,30 +161,50 @@ body {
top: 8rem;
left: 0;
right: 0;
- padding: 1rem 2rem;
+ padding: 3rem 1rem 1rem 1.5rem;
}
- .has-dropdown:not(.is-active) > .navbar-dropdown {
- display: none;
+ .navbar-item-section {
+ width: 47%;
Review comment:
Positioning of this is a bit misaligned:
Perhaps something like:
```diff
@media screen and (max-width: 1023px) {
.navbar-menu.is-active {
- padding: 3rem 1rem 1rem 1.5rem;
+ padding: 1rem;
}
.navbar-item-section {
- width: 47%;
+ width: calc(50% - 1rem);
- margin: 0 .5rem 1.5rem 0;
+ margin: 0.5rem;
}
}
```
##########
File path: antora-ui-camel/src/js/05-navbar.js
##########
@@ -10,11 +11,8 @@ document.addEventListener('DOMContentLoaded', function () {
})
})
- var menuDropDowns = Array.prototype.slice.call(document.querySelectorAll('.has-dropdown'), 0)
- menuDropDowns.forEach(function (el) {
- el.addEventListener('click', function (e) {
- e.stopPropagation()
- el.classList.toggle('is-active')
- })
+ window.addEventListener('scroll', function () {
+ if (this.scrollY <= 30) navbar.classList.remove('scrolled')
Review comment:
This is in pixels, and it need not be 30px depending on the configured user font size or browser font rendering/sizing. I think we should check if an element just below the navbar is clipped or not, [here is an article](https://css-tricks.com/styling-based-on-scroll-position/) on how to do that.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -174,30 +161,50 @@ body {
top: 8rem;
left: 0;
right: 0;
- padding: 1rem 2rem;
+ padding: 3rem 1rem 1rem 1.5rem;
}
- .has-dropdown:not(.is-active) > .navbar-dropdown {
- display: none;
+ .navbar-item-section {
+ width: 47%;
+ height: 10rem;
+ background: var(--color-camel-orange);
+ border-radius: 15px;
+ margin: 0 0.5rem 1.5rem 0;
}
- .has-dropdown:not(.is-active) > .navbar-link::after {
- transform: rotate(-45deg);
+ .navbar-item-section img {
+ display: block;
+ margin-left: calc(50% - 2.5rem);
+ margin-top: 1.5rem;
}
- .has-dropdown.is-active > .navbar-dropdown {
- display: block;
- position: relative;
- border: none;
+ .navbar-item.navbar-topics {
+ text-align: center;
+ color: var(--navbar-background);
+ text-transform: uppercase;
+ font-size: 20px;
}
- .has-dropdown.is-active > .navbar-link::after {
- transform: rotate(135deg);
+ @media screen and (min-width: 480px) and (max-width: 626px) {
+ .navbar-item.navbar-topics {
+ font-size: 18px;
+ }
}
- .navbar-item.navbar-topics,
- .navbar-link.navbar-topics {
- color: var(--navbar-font-color);
+ @media screen and (max-width: 480px) {
+ .navbar-item.navbar-topics {
+ font-size: 15px;
+ }
+ }
+
+ @media screen and (max-width: 375px) {
+ .navbar-item-section {
+ width: 45%;
+ }
Review comment:
Not sure this is needed.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -174,30 +161,50 @@ body {
top: 8rem;
left: 0;
right: 0;
- padding: 1rem 2rem;
+ padding: 3rem 1rem 1rem 1.5rem;
}
- .has-dropdown:not(.is-active) > .navbar-dropdown {
- display: none;
+ .navbar-item-section {
+ width: 47%;
+ height: 10rem;
+ background: var(--color-camel-orange);
+ border-radius: 15px;
+ margin: 0 0.5rem 1.5rem 0;
}
- .has-dropdown:not(.is-active) > .navbar-link::after {
- transform: rotate(-45deg);
+ .navbar-item-section img {
+ display: block;
+ margin-left: calc(50% - 2.5rem);
+ margin-top: 1.5rem;
}
- .has-dropdown.is-active > .navbar-dropdown {
- display: block;
- position: relative;
- border: none;
+ .navbar-item.navbar-topics {
+ text-align: center;
+ color: var(--navbar-background);
+ text-transform: uppercase;
+ font-size: 20px;
}
- .has-dropdown.is-active > .navbar-link::after {
- transform: rotate(135deg);
+ @media screen and (min-width: 480px) and (max-width: 626px) {
+ .navbar-item.navbar-topics {
+ font-size: 18px;
+ }
}
- .navbar-item.navbar-topics,
- .navbar-link.navbar-topics {
- color: var(--navbar-font-color);
+ @media screen and (max-width: 480px) {
+ .navbar-item.navbar-topics {
+ font-size: 15px;
Review comment:
We should do `rem`s for `font-size`.
##########
File path: antora-ui-camel/src/partials/header-content.hbs
##########
@@ -16,7 +16,10 @@
</div>
</div>
{{else}}
- <a class="navbar-item navbar-topics" href="{{url}}">{{name}}</a>
+ <div class="navbar-item-section">
+ <img src="{{siteRootPath}}/_/img/{{name}}.svg">
+ <a class="navbar-item navbar-topics" href="{{url}}">{{name}}</a>
+ </div>
Review comment:
Why not make this an anchor tag with an image within it, instead of div with image and anchor. Would make the whole item clickable.
----------------------------------------------------------------
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
[GitHub] [camel-website] zregvart merged pull request #448: Improvize on the style of navbar and navbar items layout
Posted by GitBox <gi...@apache.org>.
zregvart merged pull request #448:
URL: https://github.com/apache/camel-website/pull/448
----------------------------------------------------------------
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
[GitHub] [camel-website] zregvart commented on pull request #448: Improvize on the style of navbar and navbar items layout
Posted by GitBox <gi...@apache.org>.
zregvart commented on pull request #448:
URL: https://github.com/apache/camel-website/pull/448#issuecomment-671595217
This needs to be rebased, there are bits from #451 we need to take into here (the configuration from `config.toml`).
----------------------------------------------------------------
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