Add expand-headerless option to Panel component #1417

raysonkoh · 2020-12-12T05:00:23Z

What is the purpose of this pull request?

Fixes #925

Overview of changes:

Add a new option expand-headerless to the Panel component. If it is set as true, the Panel header will not be shown when the Panel is expanded, else the Panel header will be shown when the Panel is expanded. It is set as false by default.
Add test cases to test for expand-headerless behavior for both NestedPanel and MinimalPanel
Update User Guide --> Components --> Panels with explanation of expand-headerless

Anything you'd like to highlight / discuss:

Usage of v-if vs v-show: Tried to use v-show = "shouldShowHeader" but it seems to not be working. Only v-if = "shouldShowHeader" works. Would it be okay to use v-if in this case?

Testing instructions:
npm run test

Proposed commit message: (wrap lines at 72 characters) Add expand-headerless option to Panel component

Checklist: ☑️

Updated the documentation for feature additions and enhancements
Added tests for bug fixes or features
Linked all related issues
No blatantly unrelated changes
Pinged someone for a review!

ang-zeyu

Nice work on the tests!

Some nits:

docs/userGuide/syntax/panels.mbdf

ang-zeyu · 2020-12-12T06:20:15Z

packages/vue-components/src/panels/MinimalPanel.vue

@@ -3,7 +3,7 @@
    <div v-show="localMinimized" class="morph">
      <button class="morph-display-wrapper btn card-title morph-title" @click="open()">
        <slot name="_alt">
-          <slot name="header"></slot>


hmm, is this needed for minimal panels? (the header already automatically hides when expanded)

Yup this is not needed, thanks!

ang-zeyu · 2020-12-12T06:21:32Z

packages/vue-components/src/panels/PanelBase.js

@@ -80,12 +84,18 @@ export default {
    hasHeaderBool() {
      return this.$slots.header;
    },
+    expandHeaderlessBool() {
+      return toBoolean(this.expandHeaderless);


the additional toBoolean shouldn't be required since you're using type: Boolean above

Okay, I will make the changes

ang-zeyu · 2020-12-12T06:26:42Z

packages/vue-components/src/panels/NestedPanel.vue

@@ -3,7 +3,7 @@
    <div v-show="localMinimized" class="morph">
      <button :class="['morph-display-wrapper', 'btn', btnType, 'card-title']" @click="open()">
        <slot name="_alt">
-          <slot name="header"></slot>


hmm what's this for? seeing that we wouldn't have a panel expanded but minimized at the same time

Yes this is not needed, thanks for the catch!

ang-zeyu · 2020-12-12T06:32:53Z

packages/vue-components/src/panels/NestedPanel.vue

@@ -19,7 +19,7 @@
          ></span>
        </div>
        <div ref="headerWrapper" :class="['header-wrapper card-title', cardType, {'text-white':!isLightBg}]">
-          <slot name="header"></slot>


wondering if we should add some opacity transition of sorts to reduce the movement:

instead of removing it completely*

I see so maybe like transition: 0.3s opacity for the header when it is expanded or something to that effect?

Are you referring to this sort of behavior?

yup - this looks great

so the height of the header shouldn't change before / after expansion, which may be jarring for the reader

Okay thanks! I might need some more time to test this out, but I think I have addressed the rest of the PR comments.

@ang-zeyu You can check now to see whether the behavior is correct.

Co-authored-by: Ang Ze Yu <angzeyu@gmail.com>

- Remove v-if for minimized header since it is not needed - Remove usage of toBoolean in expandHeaderlessBool

ang-zeyu · 2020-12-12T13:29:37Z

packages/vue-components/src/panels/MinimalPanel.vue

@@ -19,7 +19,7 @@
            ref="headerWrapper"
            :class="['card-title', 'card-title-transparent', { 'ellipses': !hasHeaderBool }]"
          >
-            <span class="card-title-inline"><slot name="header"></slot></span>
+            <span class="card-title-inline"><slot v-if="shouldShowHeader" name="header"></slot></span>


Thanks for the catch!

ang-zeyu · 2020-12-12T13:30:08Z

packages/vue-components/src/panels/PanelBase.js

@@ -80,12 +84,18 @@ export default {
    hasHeaderBool() {
      return this.$slots.header;
    },
+    expandHeaderlessBool() {


hmm do we still need this?

This is not needed anymore I will remove it, Thanks!

ang-zeyu · 2020-12-12T13:33:05Z

packages/vue-components/src/panels/NestedPanel.vue

@@ -19,7 +19,7 @@
          ></span>
        </div>
        <div ref="headerWrapper" :class="['header-wrapper card-title', cardType, {'text-white':!isLightBg}]">
-          <slot name="header"></slot>


yup - this looks great

so the height of the header shouldn't change before / after expansion, which may be jarring for the reader

ang-zeyu · 2020-12-12T13:34:56Z

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

+  });
+});
+
+describe('MinimalPanel', () => {


let's remove the minimal panel ones since it dosen't have this feature

okay, removed!

- Remove all instances of shouldShowHeader in MinimalPanel - Remove tests for expand-headerless for MinimalPanel - Remove expandHeaderlessBool method in PanelBase

- Change opacity to 0 with transition if expand-headerless is true

ang-zeyu

Lgtm 👍

Thanks! @raysonkoh

raysonkoh added 3 commits December 12, 2020 11:20

Update panels to expand headerless

82e8008

Add tests for panels expandHeaderless

c09f31f

Update userguide for expand-headerless

d639814

ang-zeyu reviewed Dec 12, 2020

View reviewed changes

raysonkoh and others added 2 commits December 12, 2020 19:26

Apply suggestions from code review

383d70f

Co-authored-by: Ang Ze Yu <angzeyu@gmail.com>

Update code based on review

ccd27be

- Remove v-if for minimized header since it is not needed - Remove usage of toBoolean in expandHeaderlessBool

ang-zeyu reviewed Dec 12, 2020

View reviewed changes

raysonkoh added 2 commits December 12, 2020 22:07

Update according to PR comments

529e478

- Remove all instances of shouldShowHeader in MinimalPanel - Remove tests for expand-headerless for MinimalPanel - Remove expandHeaderlessBool method in PanelBase

Update behavior of expand-headerless

7c4479e

- Change opacity to 0 with transition if expand-headerless is true

ang-zeyu approved these changes Dec 12, 2020

View reviewed changes

ang-zeyu added this to the v3.0 milestone Dec 12, 2020

ang-zeyu merged commit afed1e1 into MarkBind:master Dec 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add expand-headerless option to Panel component #1417

Add expand-headerless option to Panel component #1417

raysonkoh commented Dec 12, 2020 •

edited

Loading

ang-zeyu left a comment

ang-zeyu Dec 12, 2020

raysonkoh Dec 12, 2020

ang-zeyu Dec 12, 2020

raysonkoh Dec 12, 2020

ang-zeyu Dec 12, 2020

raysonkoh Dec 12, 2020

ang-zeyu Dec 12, 2020

ang-zeyu Dec 12, 2020

raysonkoh Dec 12, 2020 •

edited

Loading

raysonkoh Dec 12, 2020 •

edited

Loading

ang-zeyu Dec 12, 2020

raysonkoh Dec 12, 2020

raysonkoh Dec 12, 2020

ang-zeyu Dec 12, 2020

raysonkoh Dec 12, 2020

ang-zeyu Dec 12, 2020

raysonkoh Dec 12, 2020

ang-zeyu Dec 12, 2020

ang-zeyu Dec 12, 2020

raysonkoh Dec 12, 2020

ang-zeyu left a comment

Add expand-headerless option to Panel component #1417

Add expand-headerless option to Panel component #1417

Conversation

raysonkoh commented Dec 12, 2020 • edited Loading

ang-zeyu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raysonkoh Dec 12, 2020 • edited Loading

Choose a reason for hiding this comment

raysonkoh Dec 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ang-zeyu left a comment

Choose a reason for hiding this comment

raysonkoh commented Dec 12, 2020 •

edited

Loading

raysonkoh Dec 12, 2020 •

edited

Loading

raysonkoh Dec 12, 2020 •

edited

Loading