Uploaded image for project: 'Camel'
  1. Camel
  2. CAMEL-14958

Suggestion on search bar within smaller screen width.

Details

    • Improvement
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • None
    • None
    • website
    • 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

        1. bootstrap-design-blog.png
          52 kB
          Aemie
        2. bootstrap-design-frontpage.png
          61 kB
          Aemie
        3. design-frontpage.png
          63 kB
          Aemie
        4. design-navbar.png
          12 kB
          Aemie
        5. inline-search.png
          10 kB
          Aemie
        6. issue-facing.png
          37 kB
          Aemie
        7. navbar-search-414.png
          10 kB
          Aemie
        8. navbar-searchbar.png
          2 kB
          Aemie
        9. nav-searchbar-1220.png
          30 kB
          Aemie
        10. nav-searchbar-414.png
          3 kB
          Aemie
        11. nav-search-icon-1220.png
          16 kB
          Aemie
        12. transform-method.png
          5 kB
          Aemie

        Issue Links

          Activity

            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            { color: var(--color-white); }

            +
            +@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

            { + display: none; + flex-basis: 100%; + height: 0; Review comment: I'm not sure that elements that have `display:none` have a hight, do they? ########## File path: antora-ui-camel/src/css/doc.css ########## @@ -37,12 +37,18 @@ margin: 1.5rem 0; }

            -@media screen and (min-width: 769px) {
            +@media screen and (min-width: 1024px) {
            .doc > h1.page:first-child

            { margin-top: 2.5rem; }

            }

            +@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

            { height: var(--navbar-height); }

            +@media screen and (max-width: 1277px) {
            + .navbar-brand

            { + flex-wrap: wrap; + }

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

            { margin-left: 2rem; }

            + @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

            { color: var(--color-white); }

            +
            +@media screen and (max-width: 1023px) {
            + article.blog:first-child

            { + margin-top: 5rem; + }

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

            { + margin-top: var(--navbar-height); + }

            +
            header.frontpage h1

            { font-size: 9vw; }

            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

            { + margin-top: var(--navbar-height); Review comment: Could be unnecessary if we move the `--body-top`. ########## File path: antora-ui-camel/src/css/static.css ########## @@ -9,6 +8,12 @@ }

            }

            +@media screen and (max-width: 1023px) {
            + .static

            { + margin-top: 5rem; Review comment: Could be unnecessary if we move the `--body-top`. ########## File path: antora-ui-camel/src/css/toc.css ########## @@ -8,6 +8,13 @@ top: var(--toc-top); }

            +@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

            { padding: 1rem 2rem 1rem 0; }

            + @media screen and (max-width: 1080px) {
            + .navbar-item

            { + padding-right: 1.5rem; + }

            + }

            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

            githubbot ASF GitHub Bot added a comment - 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 { color: var(--color-white); } + +@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 { + display: none; + flex-basis: 100%; + height: 0; Review comment: I'm not sure that elements that have `display:none` have a hight, do they? ########## File path: antora-ui-camel/src/css/doc.css ########## @@ -37,12 +37,18 @@ margin: 1.5rem 0; } -@media screen and (min-width: 769px) { +@media screen and (min-width: 1024px) { .doc > h1.page:first-child { margin-top: 2.5rem; } } +@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 { height: var(--navbar-height); } +@media screen and (max-width: 1277px) { + .navbar-brand { + flex-wrap: wrap; + } +} + .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 { margin-left: 2rem; } + @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 { color: var(--color-white); } + +@media screen and (max-width: 1023px) { + article.blog:first-child { + margin-top: 5rem; + } + + .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 { + margin-top: var(--navbar-height); + } + header.frontpage h1 { font-size: 9vw; } 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 { + margin-top: var(--navbar-height); Review comment: Could be unnecessary if we move the `--body-top`. ########## File path: antora-ui-camel/src/css/static.css ########## @@ -9,6 +8,12 @@ } } +@media screen and (max-width: 1023px) { + .static { + margin-top: 5rem; Review comment: Could be unnecessary if we move the `--body-top`. ########## File path: antora-ui-camel/src/css/toc.css ########## @@ -8,6 +8,13 @@ top: var(--toc-top); } +@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 { padding: 1rem 2rem 1rem 0; } + @media screen and (max-width: 1080px) { + .navbar-item { + padding-right: 1.5rem; + } + } 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            { height: var(--navbar-height); }

            +@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

            githubbot ASF GitHub Bot added a comment - 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 { height: var(--navbar-height); } +@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
            githubbot ASF GitHub Bot added a comment -

            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

            { height: var(--navbar-height); }

            +@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

            githubbot ASF GitHub Bot added a comment - 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 { height: var(--navbar-height); } +@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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            { padding: 0 0.375rem; }

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

            githubbot ASF GitHub Bot added a comment - 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 { padding: 0 0.375rem; } -@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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            { height: var(--navbar-height); }

            +@media screen and (max-width: 1277px) {
            + .navbar-brand

            { + flex-wrap: wrap; + }

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

            { position: relative; bottom: calc(50% - 0.15rem); - left: calc(100% - 1.25rem); + left: calc(100% - 1.5rem); height: 1rem; display: none; cursor: pointer; }

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

            { padding: 0 0.375rem; }

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

            { height: var(--navbar-height); }

            +@media screen and (max-width: 1277px) {
            + .navbar-brand

            { + flex-wrap: wrap; Review comment: I think this is causing some issues on Firefox, oddly not in the responsive tool but when the browser is resized. Not sure if we can prevent this. When the browser is resized: ![Screenshot_2020-06-26 Home - Apache Camel](https://user-images.githubusercontent.com/1306050/85838701-404f3200-b79a-11ea-8579-8cd8b66dac5a.png) In responsive tool: ![Screenshot_2020-06-26 Home - Apache Camel(1)](https://user-images.githubusercontent.com/1306050/85838691-3c231480-b79a-11ea-8af9-9a9c17c3c568.png) ########## File path: antora-ui-camel/src/css/doc.css ########## @@ -532,6 +538,19 @@ padding: 0.75rem; }

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

            { height: var(--navbar-height); }

            +@media screen and (max-width: 1277px) {
            + .navbar-brand

            { + flex-wrap: wrap; + }

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

            { height: var(--navbar-height); }

            +@media screen and (max-width: 1277px) {
            + .navbar-brand

            { + flex-wrap: wrap; + }

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

            { padding-right: 0.5rem; }

            -#search_results dt {
            +.search-results dt

            { padding: 0.5rem; color: var(--color-camel-orange); text-transform: uppercase; border-top: 1px solid var(--navbar-search-result-separator); border-bottom: 1px solid var(--navbar-search-result-separator); }

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

            { margin: 0.25rem 0; }

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

            { padding-right: 0.5rem; }

            -#search_results dt {
            +.search-results dt

            { Review comment: Now we have only one element, should we target by id instead? ########## File path: antora-ui-camel/src/css/nav.css ########## @@ -24,13 +24,19 @@ }

            }

            +@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

            { color: var(--color-jet-50); }

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

            { top: var(--navbar-height); + height: var(--nav-menu-panel-height); + }

            +
            + .nav-category

            { + top: 8.5rem; box-shadow: none; position: sticky; height: var(--nav-height--desktop); }

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

            { overflow-y: auto; }

            +@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

            { top: var(--navbar-height); + height: var(--nav-menu-panel-height); + }

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

            { padding-right: 0.5rem; }

            -#search_results dt {
            +.search-results dt

            { padding: 0.5rem; color: var(--color-camel-orange); text-transform: uppercase; border-top: 1px solid var(--navbar-search-result-separator); border-bottom: 1px solid var(--navbar-search-result-separator); }

            -#search_results a {
            +.search-results a

            { padding: 0.5rem; }

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

            githubbot ASF GitHub Bot added a comment - 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 { height: var(--navbar-height); } +@media screen and (max-width: 1277px) { + .navbar-brand { + flex-wrap: wrap; + } +} + .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 { position: relative; bottom: calc(50% - 0.15rem); - left: calc(100% - 1.25rem); + left: calc(100% - 1.5rem); height: 1rem; display: none; cursor: pointer; } -#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 { padding: 0 0.375rem; } -@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 { height: var(--navbar-height); } +@media screen and (max-width: 1277px) { + .navbar-brand { + flex-wrap: wrap; Review comment: I think this is causing some issues on Firefox, oddly not in the responsive tool but when the browser is resized. Not sure if we can prevent this. When the browser is resized: ![Screenshot_2020-06-26 Home - Apache Camel](https://user-images.githubusercontent.com/1306050/85838701-404f3200-b79a-11ea-8579-8cd8b66dac5a.png) In responsive tool: ![Screenshot_2020-06-26 Home - Apache Camel(1)](https://user-images.githubusercontent.com/1306050/85838691-3c231480-b79a-11ea-8af9-9a9c17c3c568.png) ########## File path: antora-ui-camel/src/css/doc.css ########## @@ -532,6 +538,19 @@ padding: 0.75rem; } +.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 { height: var(--navbar-height); } +@media screen and (max-width: 1277px) { + .navbar-brand { + flex-wrap: wrap; + } +} + .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 { height: var(--navbar-height); } +@media screen and (max-width: 1277px) { + .navbar-brand { + flex-wrap: wrap; + } +} + .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 { padding-right: 0.5rem; } -#search_results dt { +.search-results dt { padding: 0.5rem; color: var(--color-camel-orange); text-transform: uppercase; border-top: 1px solid var(--navbar-search-result-separator); border-bottom: 1px solid var(--navbar-search-result-separator); } -#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 { margin: 0.25rem 0; } -@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 { padding-right: 0.5rem; } -#search_results dt { +.search-results dt { Review comment: Now we have only one element, should we target by id instead? ########## File path: antora-ui-camel/src/css/nav.css ########## @@ -24,13 +24,19 @@ } } +@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 { color: var(--color-jet-50); } -.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 { top: var(--navbar-height); + height: var(--nav-menu-panel-height); + } + + .nav-category { + top: 8.5rem; box-shadow: none; position: sticky; height: var(--nav-height--desktop); } + + .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 { overflow-y: auto; } +@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 { top: var(--navbar-height); + height: var(--nav-menu-panel-height); + } + + .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 { padding-right: 0.5rem; } -#search_results dt { +.search-results dt { padding: 0.5rem; color: var(--color-camel-orange); text-transform: uppercase; border-top: 1px solid var(--navbar-search-result-separator); border-bottom: 1px solid var(--navbar-search-result-separator); } -#search_results a { +.search-results a { padding: 0.5rem; } -#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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            Aemie Aemie added a comment -

            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?

            Aemie Aemie added a comment - 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?
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            { text-align: right; }

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

            githubbot ASF GitHub Bot added a comment - 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 { text-align: right; } +.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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            { text-align: right; }

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

            githubbot ASF GitHub Bot added a comment - 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 { text-align: right; } +.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
            githubbot ASF GitHub Bot added a comment -

            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

            { margin-top: 3rem; }

            +@media screen and (max-width: 1023px) {
            + .blog.list aside

            { + margin-top: 2.5rem; + }

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

            githubbot ASF GitHub Bot added a comment - 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 { margin-top: 3rem; } +@media screen and (max-width: 1023px) { + .blog.list aside { + margin-top: 2.5rem; + } + + 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            { text-align: right; }

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

            githubbot ASF GitHub Bot added a comment - 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 { text-align: right; } +.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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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-height desktop: 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
            githubbot ASF GitHub Bot added a comment -

            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:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - 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: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

            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

            githubbot ASF GitHub Bot added a comment - 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
            githubbot ASF GitHub Bot added a comment -

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

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

                1. BEFORE TOOLBAR PANEL DESIGN
                  ![old-toolbar](https://user-images.githubusercontent.com/44139348/84804375-4e75b380-b020-11ea-81d6-30ffbd68ca0e.png)
                1. AFTER TOOLBAR PANEL DESIGN
                  ![new-toolbar-design](https://user-images.githubusercontent.com/44139348/84804388-52093a80-b020-11ea-9248-66ba2c669c5b.png)
              1. 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
            ```

                1. BASIC DESIGN IMPLEMENTATION EXAMPLE
                  ![search-bar-design](https://user-images.githubusercontent.com/44139348/84804532-8f6dc800-b020-11ea-9504-b0552a2b210b.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

            githubbot ASF GitHub Bot added a comment - 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 ) AFTER TOOLBAR PANEL DESIGN ! [new-toolbar-design] ( https://user-images.githubusercontent.com/44139348/84804388-52093a80-b020-11ea-9248-66ba2c669c5b.png ) 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 ) ---------------------------------------------------------------- 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 Zoran Regvart added a comment -

            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 Zoran Regvart added a comment - 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.
            Aemie Aemie added a comment -

            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. 

            Aemie Aemie added a comment - 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. 
            Aemie Aemie added a comment - - edited

            delawen I have resolved the issue for that. I will be creating a PR after I am done with complete implementation.

            Aemie Aemie added a comment - - edited 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?

            delawen Maria Arias de Reyna Dominguez added a comment - Do you have the netlify link for checking the tested website? Maybe those uris work on a separated set of stylesheets?
            Aemie Aemie added a comment - - edited

            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.

            Aemie Aemie added a comment - - edited 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.
            Aemie Aemie added a comment -

            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.

            Aemie Aemie added a comment - 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

            delawen Maria Arias de Reyna Dominguez added a comment - - edited 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
            Aemie Aemie added a comment -

            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? 

            Aemie Aemie added a comment - 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? 
            Aemie Aemie added a comment -

            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 Aemie added a comment - 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? 
            zregvart Zoran Regvart added a comment -

            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 Zoran Regvart added a comment - 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.
            Aemie Aemie added a comment -

            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?

            Aemie Aemie added a comment - 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 Zoran Regvart added a comment -

            Aemie looks good to me, thanks!

            zregvart Zoran Regvart added a comment - Aemie looks good to me, thanks!
            Aemie Aemie added a comment - - edited

            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

            Aemie Aemie added a comment - - edited 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
            Aemie Aemie added a comment -

            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 Aemie added a comment - 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.
            zregvart Zoran Regvart added a comment -

            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 Zoran Regvart added a comment - 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.
            Aemie Aemie added a comment -

            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? 

            Aemie Aemie added a comment - 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? 
            Aemie Aemie added a comment - - edited

            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?

            Aemie Aemie added a comment - - edited 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?
            aashnajena Aashna Jena added a comment - - edited

            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.

            aashnajena Aashna Jena added a comment - - edited 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.
            Aemie Aemie added a comment -

            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.

            Aemie Aemie added a comment - 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.
            Aemie Aemie added a comment - - edited

            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.

            Aemie Aemie added a comment - - edited 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.
            zregvart Zoran Regvart added a comment -

            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.

            zregvart Zoran Regvart added a comment - 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.
            aashnajena Aashna Jena added a comment -

            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?

            aashnajena Aashna Jena added a comment - 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?
            Aemie Aemie added a comment -

            zregvart, aashnajena what are your views on this?  The state will toggle between 1 &  2 on click. 

            Aemie Aemie added a comment - zregvart , aashnajena  what are your views on this?  The state will toggle between 1 &  2 on click. 

            People

              Unassigned Unassigned
              Aemie Aemie
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: