Panel: Add popup window button #27

yamgent · 2018-05-21T05:55:24Z

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [X] New feature

Resolves MarkBind/markbind#225

What is the rationale for this request?
User wants a way to add popup windows for panels.

What changes did you make? (Give an overview)

Add a popup window button to the Panel.
Gave Panel a new attribute popup-url, which when specified, will be the URL that the popup window button navigates to.

Provide a demo:

Is there anything you'd like reviewers to focus on?
The description of the new button in the documentation.

Testing instructions:

Navigate to the panel that has the popup window button enabled.
Click on the button. A new window should appear.

damithc · 2018-05-21T06:35:30Z

I guess you'll be adding a way to disable it too? we have no-close etc. for other buttons.

yamgent · 2018-05-21T06:50:26Z

I guess you'll be adding a way to disable it too? we have no-close etc. for other buttons.

Unlike the close and expand button, which is enabled by default, the popup button isn't enabled by default (because it relies on a provided URL for the popup to work, and by default such URL is not provided).

acjh · 2018-05-21T06:52:52Z

Let's not reuse the term "popup", ~~since this is very different from the Popup component.~~

Edit:
The component is Popover. "popup" is still not quite "new window/tab", but I guess it's fine.

Chng-Zhi-Xuan

Feature works great, just need to focus on clarity and wording to Users.

Chng-Zhi-Xuan · 2018-05-21T07:20:14Z

docs/example/accordionDocs.vue

+        <p>popup-url</p>
+        <p><code>String</code></p>
+        <p></p>
+        <p>The url to display in the popup window.</p>


Should highlight to the User that either a remote web url or local directory path can be given.

Chng-Zhi-Xuan · 2018-05-21T07:21:34Z

docs/example/accordionDocs.vue

@@ -95,6 +95,20 @@
    <doc-code language="markup">
      <panel header="Dynamic Loading" src="/docs/loadContent.html#fragment" minimized></panel>
    </doc-code>
+    <br>
+
+    <h4>If <code>popup-url</code> attribute is provided, the panel shows a popup button, which when clicked, opens a new window to the specified url.</h4>


Suggestion:
" ... is provided, a popup button will be displayed. Upon clicking, it opens a new window to the contents of the specified url or directory path."

It's actually still an (absolute, root-relative or relative) URL.

acjh · 2018-05-21T09:36:25Z

docs/example/accordionDocs.vue

    <div class="bs-example">
      <panel header="Some panel" popup-url="/docs/loadContent.html">
        This panel has a popup.
      </panel>
    </div>
    <doc-code language="markup">
      <panel header="Some panel" popup-url="/docs/loadContent.html">
-        ...
+        <div class="."></div>


Oops, my apologies, not intentional (not sure how it got there, WebStorm expanded it? :P)

yamgent · 2018-05-21T23:43:24Z

Minor error in the documentation fixed.

acjh · 2018-05-22T17:23:09Z

src/Panel.vue

@@ -34,6 +34,10 @@
                                @click.stop="close()">
                            <span class="glyphicon glyphicon-remove" aria-hidden="true"></span>
                        </button>
+                        <button type="button" class="popup-button btn btn-default"
+                                v-show="this.popupUrl !== null" @click.stop="openPopup()">


@click.stop on new line.

acjh · 2018-05-22T17:28:15Z

docs/example/accordionDocs.vue

+        <p><code>String</code></p>
+        <p></p>
+        <p>The url that the popup window will navigate to. The url can be absolute, relative or root-relative.</p>
+      </div>


Move after bottom-switch for now (switch options are grouped).

acjh · 2018-05-22T17:30:46Z

docs/example/accordionDocs.vue

+
+    <h4>If <code>popup-url</code> attribute is provided, a popup button will be displayed. Upon clicking, it opens a new window to the specified url.</h4>
+    <div class="bs-example">
+      <panel header="Some panel" popup-url="/docs/loadContent.html">


Root-relative url likely won't work when published on GitHub. See 7dcb507.

acjh · 2018-05-22T17:31:08Z

docs/example/accordionDocs.vue

+    </div>
+    <doc-code language="markup">
+      <panel header="Some panel" popup-url="/docs/loadContent.html">
+        ...


For simple content, show as-is.

acjh · 2018-05-22T17:33:55Z

docs/example/accordionDocs.vue

@@ -95,6 +95,20 @@
    <doc-code language="markup">
      <panel header="Dynamic Loading" src="/docs/loadContent.html#fragment" minimized></panel>
    </doc-code>
+    <br>
+
+    <h4>If <code>popup-url</code> attribute is provided, a popup button will be displayed. Upon clicking, it opens a new window to the specified url.</h4>


How about:

"will be shown" for consistent grammar in this docs.

"If clicked, it opens the specified url in a new window."

yamgent changed the title ~~Panel: Add popup button~~ Panel: Add popup window button May 21, 2018

yamgent force-pushed the 225-open-new-window branch from 774ea2e to 25ebaf5 Compare May 21, 2018 06:26

yamgent requested a review from Chng-Zhi-Xuan May 21, 2018 06:27

Chng-Zhi-Xuan reviewed May 21, 2018

View reviewed changes

acjh reviewed May 21, 2018

View reviewed changes

yamgent force-pushed the 225-open-new-window branch from 36f259c to a2639f7 Compare May 21, 2018 23:42

acjh reviewed May 22, 2018

View reviewed changes

yamgent force-pushed the 225-open-new-window branch from a2639f7 to 48abc9a Compare May 23, 2018 02:00

yamgent added 2 commits May 23, 2018 10:01

Panel: Add popup window button

1a52517

Improve documentation for popup window button

ff9d50c

yamgent force-pushed the 225-open-new-window branch from da47913 to ff9d50c Compare May 23, 2018 02:01

acjh approved these changes May 23, 2018

View reviewed changes

acjh merged commit 736bede into MarkBind:master May 24, 2018

acjh added this to the v1.1.37-markbind.8 milestone May 24, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Panel: Add popup window button #27

Panel: Add popup window button #27

yamgent commented May 21, 2018 •

edited

Loading

damithc commented May 21, 2018

yamgent commented May 21, 2018

acjh commented May 21, 2018 •

edited

Loading

Chng-Zhi-Xuan left a comment

Chng-Zhi-Xuan May 21, 2018

Chng-Zhi-Xuan May 21, 2018

acjh May 21, 2018

acjh May 21, 2018

yamgent May 21, 2018

yamgent commented May 21, 2018

acjh May 22, 2018

acjh May 22, 2018

acjh May 22, 2018

acjh May 22, 2018

acjh May 22, 2018

Panel: Add popup window button #27

Panel: Add popup window button #27

Conversation

yamgent commented May 21, 2018 • edited Loading

damithc commented May 21, 2018

yamgent commented May 21, 2018

acjh commented May 21, 2018 • edited Loading

Chng-Zhi-Xuan 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

yamgent commented May 21, 2018

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

yamgent commented May 21, 2018 •

edited

Loading

acjh commented May 21, 2018 •

edited

Loading