Skip to content
This repository was archived by the owner on Mar 26, 2022. It is now read-only.

Conversation

@flora8984461
Copy link
Contributor

@flora8984461 flora8984461 commented Oct 19, 2020

Fix #350

Updated Jan 19, 2021:

Testing pages:
https://deploy-preview-539--eclipsefdn-iot.netlify.app/community/resources/
https://deploy-preview-539--eclipsefdn-iot.netlify.app/community/
https://deploy-preview-539--eclipsefdn-iot.netlify.app/adopters/how-to-be-listed-as-an-adopter/
https://deploy-preview-539--eclipsefdn-iot.netlify.app/projects/getting-started/

And I also did some adjustments in the original sidebar.html, I removed the "row" margin in sub-sub-menu items which causes extra right spaces on mobile.

And I modified a little logic since I don't know why in config.toml we need to create duplicate sidebars.

And I removed an index.js on membership page which does not exist and caused console log error.


Old blabla:
What I have done so far:

TODO:
I probably go for collapsible, but it brings me to think of a question, For IOT site, it's kinda special since it has a sub parent menu(resources) who also have a link to go to. If using collapsible, we could not go to the link of resources in collapsible, but since it stays on the main menu, it's okay for this.

As long as we do not have cases that parent menu also has a link to go, I think collapsible works well.
The other way is to keep expanded of sub-children. Just hide or show them as a whole.

But if using dropdown, I do not need to use "", just a normal option, so that resources can be clicked and redirect to. It's just a matter of styling.

What do you think?

BTW, I also wanna add a para to control whether to show mobile-sidebar.

Signed-off-by: Yi Liu yi.liu@eclipse-foundation.org

@netlify
Copy link

netlify bot commented Oct 19, 2020

Deploy preview for eclipsefdn-iot ready!

Built with commit 82c6a69

https://deploy-preview-539--eclipsefdn-iot.netlify.app

@flora8984461 flora8984461 changed the title initial trial initial trial on improve sidebar on mobile Oct 19, 2020
@flora8984461 flora8984461 changed the title initial trial on improve sidebar on mobile improve the sidebar menu #350 Oct 19, 2020
@flora8984461 flora8984461 changed the title improve the sidebar menu #350 Improve the sidebar menu #350 Oct 19, 2020
@flora8984461
Copy link
Contributor Author

This is what I think would be better on mobile how it looks like.
I need somehow to improve the code to make it less messy.

# url = "/community/resources"
# pre = "<i data-feather=\"folder\"></i>"
# weight = 4

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the changes here,
I was confused by the previous logic that we need to create middle-layer sidebar menus twice in order to show on the page so that I made some modifications to just create once. Please also refer to the changes I made in "layouts/partials/sidebar.html".

This gives me a better understanding to create the mobile-sidebar.

Comment on lines 23 to 51
<div class="container">
<div class="row">
<div class="col-sm-24 hidden-lg hidden-md visible-sm visible-xs">
{{ range .Site.Menus.sidebar }}
{{$parentItem := . }}
{{ if and (.HasChildren) (eq $currentSection $parentItem.Identifier) }}
<select id="sub-menu" name="sub-menu">
<option>Navigate to...</option>
{{ range .Children }}
{{ if and (or (eq $parentItem.Identifier $currentSection) (eq .Parent $currentSection)) (eq .HasChildren false) }}
<option value="{{ .URL }}">
{{ .Name }}
</option>
{{ end }}
{{ if .HasChildren }}
<optgroup label="{{ .Name }}">
{{ range .Children }}
<option value="{{ .URL }}"> {{ .Name }} </option>
{{ end }}
</optgroup>
{{ end }}

{{ end }} <!-- range Children end -->
</select>
{{ end }} <!-- if and (.HasChildren) (eq $currentSection $parentItem.Identifier) end -->
{{ end }} <!-- range .Site.Menus.sidebar end -->
</div>
</div>
</div>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are a dropdown test. Just for your reference to make comparison.

Comment on lines 54 to 98
<!------------------------------------------------------------------>

<div id="mobile-sidebar" class="container hidden-lg hidden-md visible-sm visible-xs">
<div class="container sidebar-item related-content">
<a href="#sidebars-contents" data-toggle="collapse" aria-expanded="false" aria-controls="sidebars-contents">
<div class="col-xs-6"><i data-feather="plus"></i></div>
<div class="col-xs-18 sidebar-text">Related Content</div>
</a>
</div>
{{ range .Site.Menus.sidebar }}
{{$parentItem := . }}
{{ if and (.HasChildren) (eq $currentSection $parentItem.Identifier) }}
<div class="container collapse" id="sidebars-contents">
{{ range .Children }}
{{ if and (or (eq $currentSection $parentItem.Identifier) (eq .Parent $currentSection)) (eq .HasChildren false) }}
<div class="row sidebar-item{{ if eq $currentPage.Title .Name }} active{{ end }}">
<a href="{{ .URL }}" {{ if eq $currentPage.Title .Name }}class="active"{{ end }}>
<div class="col-xs-6">{{ .Pre }}</div>
<div class="col-xs-18 sidebar-text">{{ .Name }}</div>
</a>
</div>
{{ end }}
{{ if .HasChildren }}
<div class="row sidebar-item{{ if eq $currentPage.Title .Name }} active{{ end }}">
<a href="#sub-children-{{ .Identifier }}" data-toggle="collapse" aria-expanded="false" aria-controls="sub-children{{ .Identifier }}" {{ if eq $currentPage.Title .Name }}class="active"{{ end }}>
<div class="col-xs-6">{{ .Pre }}</div>
<div class="col-xs-18 sidebar-text">{{ .Name }}</div>
</a>
</div>
<div class="row sidebar-sub-items collapse" id="sub-children-{{ .Identifier }}">
{{ range .Children }}
<div class="row sidebar-sub-item{{ if eq $currentPage.Title .Name }} active{{ end }}">
<a href="{{ .URL }}" {{ if eq $currentPage.Title .Name }}class="active"{{ end }}>
<div class="col-xs-6"></div>
<div class="col-xs-18 sidebar-text">{{ .Name }}</div>
</a>
</div>
{{ end }} <!--Children's Children end-->
</div>
{{ end }}
{{ end }} <!-- range Children end -->
</div>
{{ end }} <!-- if and (.HasChildren) (eq $currentSection $parentItem.Identifier) end -->
{{ end }} <!-- range .Site.Menus.sidebar end -->
</div> <!-- "main-sidebar" end -->
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are for Collapsible test.

Comment on lines -23 to -26
{{ end }}

{{ define "footer-suffix" }}
<script src="/js/index.js"></script>
Copy link
Contributor Author

@flora8984461 flora8984461 Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what this index.js is, and it causes a console log err.

@flora8984461 flora8984461 marked this pull request as ready for review November 9, 2020 19:48
});
})(jQuery, document);

jQuery(document).ready(function($){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why you need to do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for dropdown mobile sidebar, I created 2, one is dropdown and the other is collapsible, and I will defer it to the team to choose one, and I will drop the other one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a place where both options are being displayed so that we can make a decision?

Copy link
Contributor Author

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the collapsable option but I think it would need a down arrow or something. It took me a minute to understand that this was the menu. Or maybe the hamburger icon. Something that would tell the users that this is a menu or that there is more options available to click.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decide to use a hamburger like the menu.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But for resources, I'm using a pre that is the same as the normal sidebar

<script src="//cdnjs.cloudflare.com/ajax/libs/list.js/1.2.0/list.js"></script>
<script src="//cdnjs.cloudflare.com/ajax/libs/numeral.js/1.4.5/numeral.min.js"></script>
<script src="/assets/js/projects.min.js"></script>
<script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is something you need to do more than once, can you create a partial for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I add it here because projects re-defined the footer_suffix which causes mine cannot be included.
But again, this is only for dropdown, if we decide to use dropdown, I will add it to footer.html; if we decide to use collapsible I will remove it.

@@ -0,0 +1,82 @@

#mobile-sidebar {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these probably will be updated after Eric add sidebar styles into hugo theme

@ericpoirier ericpoirier self-requested a review November 23, 2020 13:44
@flora8984461 flora8984461 force-pushed the yiliu/master/improveSidebar branch from 87ef040 to f94ef0c Compare November 25, 2020 19:45
@flora8984461 flora8984461 force-pushed the yiliu/master/improveSidebar branch from 0dd94b1 to 96af601 Compare December 18, 2020 17:45
@chrisguindon
Copy link
Member

A lot has changed since we started working on this... @flora8984461 What is the current status, is it ready to be reviewed and deploy, or is this still a WIP?

@flora8984461 flora8984461 force-pushed the yiliu/master/improveSidebar branch from 96af601 to 0edf065 Compare January 19, 2021 21:07
@flora8984461
Copy link
Contributor Author

For now, I think it's ready for review, though containing similar styles as in EclipseFdn/solstice-assets#197, but we can adjust styles later once Eric finishes that branch. But we can review and make this live if all good.

A lot has changed since we started working on this... @flora8984461 What is the current status, is it ready to be reviewed and deploy, or is this still a WIP?

Yi Liu added 5 commits February 26, 2021 11:53
Signed-off-by: Yi Liu <yi.liu@eclipse-foundation.org>
Signed-off-by: Yi Liu <yi.liu@eclipse-foundation.org>
Signed-off-by: Yi Liu <yi.liu@eclipse-foundation.org>
…ile view, and fix projects page mobilesidebar not working

Signed-off-by: Yi Liu <yi.liu@eclipse-foundation.org>
Signed-off-by: Yi Liu <yi.liu@eclipse-foundation.org>
Yi Liu added 6 commits February 26, 2021 11:53
Signed-off-by: Yi Liu <yi.liu@eclipse-foundation.org>
Signed-off-by: Yi Liu <yi.liu@eclipse-foundation.org>
Signed-off-by: Yi Liu <yi.liu@eclipse-foundation.org>
Signed-off-by: Yi Liu <yi.liu@eclipse-foundation.org>
Signed-off-by: Yi Liu <yi.liu@eclipse-foundation.org>
Signed-off-by: Yi Liu <yi.liu@eclipse-foundation.org>
@flora8984461 flora8984461 force-pushed the yiliu/master/improveSidebar branch from bfb8706 to ced775e Compare February 26, 2021 16:57
Yi Liu added 2 commits February 26, 2021 15:00
Signed-off-by: Yi Liu <yi.liu@eclipse-foundation.org>
Signed-off-by: Yi Liu <yi.liu@eclipse-foundation.org>
@flora8984461
Copy link
Contributor Author

Before finalize, I wonder if you can see the visited link color as I do or it's only me? Should we keep this visited link color?

image

@chrisguindon
Copy link
Member

I don't think we want to use a different visited link color for the sidebar.

I see this as the same as the main menu. We don't have any styles for visited links in the main menu.

@flora8984461
Copy link
Contributor Author

!!!Note: Need to update assets CSS in order to remove the visited link color before working on this

Signed-off-by: Eric Poirier <eric.poirier@eclipse-foundation.org>
@chrisguindon chrisguindon merged commit f2866a9 into EclipseFdn:master Mar 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve the sidebar menu

3 participants