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 {
font-weight: var(--body-font-weight-bold);
}
-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 {
text-align: right;
}
+.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 {
outline: 0;
}
+.navbar-small input {
+ width: 93vw;
+}
+
+@media screen and (max-width: 1024px) {
+ .navbar-small input {
+ width: 90vw;
+ }
+}
+
+@media screen and (min-width: 425px) and (max-width: 575px) {
+ .navbar-small input {
+ width: 85vw;
+ }
+}
+
+@media screen and (max-width: 425px) {
+ .navbar-small input {
+ width: 80vw;
+ }
+}
+
.navbar-search input:focus-within {
background-color: #fff;
border-color: #eaeaec;
Review comment:
(not part of this change), we should have used variables here
##########
File path: antora-ui-camel/src/css/toc.css
##########
@@ -51,11 +51,11 @@
margin: 0;
}
-.toc .toc-menu li[data-level="2"] a {
+.toc .toc-menu li[data-level='2'] a {
padding-left: 1.25rem;
}
-.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 {
margin-top: 3rem;
}
+@media screen and (max-width: 1023px) {
+ .blog.list aside {
+ margin-top: 2.5rem;
+ }
+
+ article.blog-post:first-child {
Review comment:
Should this be prefixed with `.blog`?
##########
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:
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 {
+ margin: 4.05556rem 0 1.5rem 0;
Review comment:
These are very specific numbers, I wonder if we could place them in
`vars.css` and derive their values. Think about when someone needs to edit
this, they don't have the context as to why this values have 5 decimal places.
##########
File path: antora-ui-camel/src/css/doc.css
##########
@@ -606,12 +625,12 @@
}
.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 {
outline: 0;
}
+.navbar-small input {
+ width: 93vw;
+}
+
+@media screen and (max-width: 1024px) {
+ .navbar-small input {
+ width: 90vw;
+ }
+}
+
+@media screen and (min-width: 425px) and (max-width: 575px) {
+ .navbar-small input {
+ width: 85vw;
+ }
+}
+
+@media screen and (max-width: 425px) {
+ .navbar-small input {
+ width: 80vw;
+ }
+}
Review comment:
I'd either use percentages or rem.
##########
File path: antora-ui-camel/src/css/header.css
##########
@@ -426,9 +465,9 @@ body {
cursor: pointer;
}
-#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 {
text-align: right;
}
+.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:
us...@infra.apache.org
