Add CollapseExpandButtons component #2206

itsyme · 2023-03-12T13:08:48Z

What is the purpose of this pull request?

Overview of changes:
Resolves #655. Added a CollapseExpandButtons component that renders 2 buttons to collapse and expand all dropdowns in the SiteNav.

Anything you'd like to highlight/discuss:
Looking for suggestions to improve the look of the buttons as it may look slightly out of place now. Wondering if it should be toggle-able or only appear when there are dropdowns in site-nav.

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)
Add CollapseExpandButtons component

Checklist: ☑️

Updated the documentation for feature additions and enhancements
Added tests for bug fixes or features
Linked all related issues
No unrelated changes

jovyntls · 2023-03-12T14:15:41Z

Some guidelines we could consider:

Unobtrusive - preferably not "in my face" since the focus should be on our SiteNav contents
Takes up minimal vertical space on top - we can try to keep the SiteNav contents as high up as possible since a user scanning the site nav starts from top to bottom. In other words, I think it would be good to avoid "pushing down" Sitenav contents
Would also be nice if authors can choose to hide/show the buttons on the SiteNav

Some ideas:

Or we could style it more minimalist-y, similar to our current dropdown style, and have a single arrow beside the Sitenav title ("User Guide" in this case); it functions as an "Expand all" button initially, and when clicked it toggles between "Expand all" and "Collapse all"

At the bottom:

itsyme · 2023-03-12T15:37:35Z

Hi @jovyntls! Thanks for the feedback! I will try to implement the suggestions in due course. On the look of the buttons, I really like the first 3 implementations, however the location of the buttons are in the site-nav-top which iirc is optional. I will try implementing the buttons at the bottom. Any thoughts?

jovyntls · 2023-03-12T16:26:06Z

@itsyme What do you think about making these buttons a component (like the scroll to top button)? This could allow users to choose whether or not they want to have the Expand/Collapse All buttons and give us more flexibility on where it can be added (including in optional components).

itsyme · 2023-03-12T16:50:55Z

@itsyme What do you think about making these buttons a component (like the scroll to top button)? This could allow users to choose whether or not they want to have the Expand/Collapse All buttons and give us more flexibility on where it can be added (including in optional components).

I think thats a great idea! I will try to implement that soon!

itsyme · 2023-04-02T10:36:21Z

This looks nice as a component! Not sure about having the component at the bottom with the up/down triangle arrows as it looks like it will trigger a scroll instead of a collapseAll/expandAll feature 😅

Hmmm I actually thought the up/down triangle arrows are not bad because its' consistent with the open and close button in the site nav.
I kind of prefer that over the current icons in the updated video @itsyme 😅 but ok I see it looks very similiar to the current scroll up button

What do you think about these icons instead @yucheng11122017?

How about the plus and minus button that was mentioned by @jovyntls in her first comment?

tbh if its at the top, would the triangles still seem like a scroll top scroll bottom button?

I felt like the plus and minus icons were a little too different from our present icons which are mostly from fontawesome, I will find a similar icon from fontawesome compare them!

itsyme · 2023-04-02T10:39:30Z

These are some similar icons from fontawesome

yucheng11122017 · 2023-04-02T10:48:12Z

These are some similar icons from fontawesome

Ooo I think these look a lot nicer! I think 2 fits with the site nav design :)

jovyntls · 2023-04-02T12:01:47Z

This looks nice as a component! Not sure about having the component at the bottom with the up/down triangle arrows as it looks like it will trigger a scroll instead of a collapseAll/expandAll feature 😅

Hmmm I actually thought the up/down triangle arrows are not bad because its' consistent with the open and close button in the site nav.

I agree! I have no issues with the icons - more concerned about placing it at the bottom which would be disconnected from a shorter sitenav. Having it at the top right of the site nav looks good :) Sorry for the confusion

jovyntls

The code and tests LGTM! 😄 Remaining nits are on the documentation.

I'm not sure if this component should be under "others" or "navigation" (I can see how it may fall into either 🤔 ) Though I think it would be nice to add a note about this component under the "sitenav" section to improve feature discoverability.

Also, "Collapse/Expand All Buttons" or "Collapse & Expand All Buttons" or "Collapse Expand All Buttons" be a nicer reader-facing name?

itsyme · 2023-04-03T03:12:33Z

The code and tests LGTM! 😄 Remaining nits are on the documentation.

I'm not sure if this component should be under "others" or "navigation" (I can see how it may fall into either 🤔 ) Though I think it would be nice to add a note about this component under the "sitenav" section to improve feature discoverability.

Also, "Collapse/Expand All Buttons" or "Collapse & Expand All Buttons" or "Collapse Expand All Buttons" be a nicer reader-facing name?

Thanks for reviewing my PR @jovyntls! I feel that the component fits better under "others" as the effect of the buttons do not offer any navigation functionality. I will add in a note under sitenav for this component! As for the naming, I think "Collapse/Expand All Buttons" sounds the most natural to me! I will update it in the next commit!

jovyntls

LGTM! Thank you @itsyme

yucheng11122017 · 2023-04-03T06:38:36Z

Hi @itsyme, the documentation has an example but the buttons seem to collapse and expand the site nav at the side instead of at the example.

Understand that its part of the implementation but was thinking that it would be good to not have this example in that case and just say there is an example of the left of the page at the site nav.

What do you think?

itsyme · 2023-04-03T07:05:09Z

Hi @itsyme, the documentation has an example but the buttons seem to collapse and expand the site nav at the side instead of at the example.

Understand that its part of the implementation but was thinking that it would be good to not have this example in that case and just say there is an example of the left of the page at the site nav.

What do you think?

I agree! I will make the appropriate changes in the next commit!

yucheng11122017

LGTM except for one small nit! thanks for the work

packages/vue-components/src/__tests__/CollapseExpandButtons.spec.js

yucheng11122017 · 2023-04-04T10:07:28Z

docs/userGuide/syntax/collapseExpandButtons.md

+<!-- Included in readerFacingFeatures.md -->
+<div id="examples" class="d-none">
+
+You can see an example of collapse/expand all buttons ==on the top right== of the site navigation.


Suggested change

You can see an example of collapse/expand all buttons ==on the top right== of the site navigation.

You can see an example of collapse/expand all buttons ==on the top right== of the site navigation that is located on the left of this page.

yucheng11122017 · 2023-04-04T10:12:21Z

packages/vue-components/src/CollapseExpandButtons.vue

+      class="expand-all-button"
+      @click="expandAll()"
+    >
+      <i :class="['far fa-plus-square']"></i>


I think you don't need :class here because its static not dynamic :) Same for below!

ang-zeyu · 2023-04-27T09:53:38Z

packages/vue-components/src/CollapseExpandButtons.vue

+    /* Button Positioning */
+
+    .button-container {
+        position: absolute;


The design looks nice 👍 From a product standpoint this feels a little odd for a component as it is far from standalone.

Did we consider baking this into the <site-nav>? (as an attribute or built-in)

The design may have to be tweaked slightly but this PR should largely be reusable.

We should consider this carefully before releasing

Add expand and collapse all buttons in site-nav

df38a5f

Remove warn

056495c

itsyme added 7 commits March 21, 2023 18:41

Merge branch 'master' into collapse-all

f95b9dd

Componentify CollapseExpandButtons

2cbdcbe

Merge branch 'master' into collapse-all

d537562

Remove from sitenav

0430d72

Merge branch 'master' into collapse-all

2ce2c20

Update CollapseExpandButtons

f6247cc

Merge branch 'master' into collapse-all

0c27691

itsyme changed the title ~~Add expand and collapse all buttons in site-nav~~ Add expand and collapse all button component for site-nav Mar 31, 2023

itsyme changed the title ~~Add expand and collapse all button component for site-nav~~ Add expand and collapse all button component for SiteNav Mar 31, 2023

itsyme changed the title ~~Add expand and collapse all button component for SiteNav~~ Add CollapseExpandButtons component for SiteNav Mar 31, 2023

itsyme added 2 commits March 31, 2023 23:07

Create tests

c21a5ed

Update docs

c2fe890

itsyme marked this pull request as ready for review April 1, 2023 02:21

itsyme added 11 commits April 2, 2023 08:09

Merge master

0194dd0

Resolve conflict

c321c3d

Organize fullSyntaxReference

a39be3a

Fix typo

7dd2a6d

Update docs

7be1e6b

Rename file

6507ff5

Rename file

8df94ac

Update docs

a6e13c3

Add collapse-expand-buttons to default template

850e1fb

Update tests

ddca17a

Update docs

3948a71

Change icons

6eab8d3

itsyme added 4 commits April 2, 2023 22:08

Update docs

c538778

Update positioning

0038de4

Update docs

e77d6c6

Update docs

f58cb40

jovyntls reviewed Apr 3, 2023

View reviewed changes

Update docs

56a163d

itsyme requested a review from jovyntls April 3, 2023 05:10

jovyntls approved these changes Apr 3, 2023

View reviewed changes

itsyme and others added 2 commits April 3, 2023 15:19

Update docs

d377639

Merge branch 'master' into collapse-all

0d85e0e

yucheng11122017 approved these changes Apr 4, 2023

View reviewed changes

packages/vue-components/src/__tests__/CollapseExpandButtons.spec.js Show resolved Hide resolved

yucheng11122017 reviewed Apr 4, 2023

View reviewed changes

yucheng11122017 self-requested a review April 4, 2023 10:09

yucheng11122017 reviewed Apr 4, 2023

View reviewed changes

itsyme and others added 2 commits April 4, 2023 19:32

Fix nits

65a680e

Merge branch 'master' into collapse-all

d58a2e1

itsyme changed the title ~~Add CollapseExpandButtons component for SiteNav~~ Add CollapseExpandButtons component Apr 4, 2023

itsyme merged commit 61eebde into MarkBind:master Apr 4, 2023

ang-zeyu reviewed Apr 27, 2023

View reviewed changes

ang-zeyu added this to the v5.0.0 milestone Apr 27, 2023

ang-zeyu mentioned this pull request Apr 27, 2023

Should CollapseExpandButtons component be built into site-nav? #2299

Closed

jovyntls mentioned this pull request Jun 6, 2023

Revert expand collapse all buttons #2303

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CollapseExpandButtons component #2206

Add CollapseExpandButtons component #2206

itsyme commented Mar 12, 2023 •

edited

Loading

jovyntls commented Mar 12, 2023

itsyme commented Mar 12, 2023

jovyntls commented Mar 12, 2023 •

edited

Loading

itsyme commented Mar 12, 2023

itsyme commented Apr 2, 2023

itsyme commented Apr 2, 2023

yucheng11122017 commented Apr 2, 2023

jovyntls commented Apr 2, 2023 •

edited

Loading

jovyntls left a comment

itsyme commented Apr 3, 2023 •

edited

Loading

jovyntls left a comment

yucheng11122017 commented Apr 3, 2023

itsyme commented Apr 3, 2023

yucheng11122017 left a comment

yucheng11122017 Apr 4, 2023

yucheng11122017 Apr 4, 2023

ang-zeyu Apr 27, 2023

	You can see an example of collapse/expand all buttons ==on the top right== of the site navigation.
	You can see an example of collapse/expand all buttons ==on the top right== of the site navigation that is located on the left of this page.

Add CollapseExpandButtons component #2206

Add CollapseExpandButtons component #2206

Conversation

itsyme commented Mar 12, 2023 • edited Loading

jovyntls commented Mar 12, 2023

itsyme commented Mar 12, 2023

jovyntls commented Mar 12, 2023 • edited Loading

itsyme commented Mar 12, 2023

itsyme commented Apr 2, 2023

itsyme commented Apr 2, 2023

yucheng11122017 commented Apr 2, 2023

jovyntls commented Apr 2, 2023 • edited Loading

jovyntls left a comment

Choose a reason for hiding this comment

itsyme commented Apr 3, 2023 • edited Loading

jovyntls left a comment

Choose a reason for hiding this comment

yucheng11122017 commented Apr 3, 2023

itsyme commented Apr 3, 2023

yucheng11122017 left a comment

Choose a reason for hiding this comment

yucheng11122017 Apr 4, 2023

Choose a reason for hiding this comment

yucheng11122017 Apr 4, 2023

Choose a reason for hiding this comment

ang-zeyu Apr 27, 2023

Choose a reason for hiding this comment

itsyme commented Mar 12, 2023 •

edited

Loading

jovyntls commented Mar 12, 2023 •

edited

Loading

jovyntls commented Apr 2, 2023 •

edited

Loading

itsyme commented Apr 3, 2023 •

edited

Loading