Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove OK button from modals #1134

Merged
merged 1 commit into from
Mar 23, 2020
Merged

Remove OK button from modals #1134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions docs/userGuide/syntax/modals.mbdf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@ This is the same <trigger for="modal:loremipsum">trigger</trigger> as last one.
Centered
</modal>

<trigger for="modal:ok-text">This is a trigger for a modal with a custom OK button</trigger>.

<modal header="OK button visible!" id="modal:ok-text" ok-text="Custom OK">
Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore
magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo
consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
</modal>

```
</span>
<span id="output">
Copy link
Contributor

Choose a reason for hiding this comment

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

should ok-text be ok-title instead? ( the props table )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be fine as ok-text. since it's not really a title of anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the redundant OK button from modals, only show it if the author specifies ok-title.

Ahh ok, thought the attribute from the user standpoint here was ok-title

Expand All @@ -41,6 +50,15 @@ This is the same <trigger for="modal:loremipsum">trigger</trigger> as last one.
<modal header="**Centered** :rocket:" id="modal:centered" center>
Centered
</modal>

<trigger for="modal:ok-text">This is a trigger for a modal with a custom OK button</trigger>.

<modal header="OK button visible!" id="modal:ok-text" ok-text="Custom OK">
Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore
magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo
consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
</modal>
</span>
</include>

Expand Down
13 changes: 12 additions & 1 deletion src/lib/markbind/src/parsers/componentParser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,18 @@ function _parseModalAttributes(node) {
_renameAttribute(node, 'ok-text', 'ok-title');
_renameAttribute(node, 'center', 'centered');

node.attribs['ok-only'] = ''; // only show OK button
const hasOkTitle = _.has(node.attribs, 'ok-title');
const hasFooter = node.children.some(child =>
_.has(child.attribs, 'slot') && child.attribs.slot === 'modal-footer');

if (!hasFooter && !hasOkTitle) {
// markbind doesn't show the footer by default
node.attribs['hide-footer'] = '';
Copy link
Contributor

Choose a reason for hiding this comment

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

does setting it to true erase the ="" portion in hide-footer=""? might be nice if so, same for ok-only

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, i just get this

image

} else if (hasOkTitle) {
// bootstrap-vue default is to show ok and cancel
// if there's an ok-title, markbind only shows the OK button.
node.attribs['ok-only'] = '';
}

if (node.attribs.backdrop === 'false') {
node.attribs['no-close-on-backdrop'] = '';
Expand Down
4 changes: 2 additions & 2 deletions test/functional/test_site/expected/bugs/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
<p>Repro:</p>
<div>
<p>This is to reproduce <span trigger="click" for="modal:bugRepro" v-b-popover.click.top.html="popoverGenerator" v-b-tooltip.click.top.html="tooltipContentGetter" v-on:click="$refs['modal:bugRepro'].show()" class="trigger">multiple inclusions of a modal bug</span></p>
<b-modal id="modal:bugRepro" ok-only="" size="lg" modal-class="mb-zoom" ref="modal:bugRepro"><template slot="modal-title">Establishing Requirements</template>
<b-modal id="modal:bugRepro" hide-footer="" size="lg" modal-class="mb-zoom" ref="modal:bugRepro"><template slot="modal-title">Establishing Requirements</template>
<div>
<p>Requirements gathering, requirements elicitation, requirements analysis, requirements capture are some of the terms commonly <strong>and</strong> interchangeably used to represent the activity of understanding what a software product should
do.
Expand All @@ -60,7 +60,7 @@
</div>
<div>
<p>This is to reproduce <span trigger="click" for="modal:bugRepro" v-b-popover.click.top.html="popoverGenerator" v-b-tooltip.click.top.html="tooltipContentGetter" v-on:click="$refs['modal:bugRepro'].show()" class="trigger">multiple inclusions of a modal bug</span></p>
<b-modal id="modal:bugRepro" ok-only="" size="lg" modal-class="mb-zoom" ref="modal:bugRepro"><template slot="modal-title">Establishing Requirements</template>
<b-modal id="modal:bugRepro" hide-footer="" size="lg" modal-class="mb-zoom" ref="modal:bugRepro"><template slot="modal-title">Establishing Requirements</template>
<div>
<p>Requirements gathering, requirements elicitation, requirements analysis, requirements capture are some of the terms commonly <strong>and</strong> interchangeably used to represent the activity of understanding what a software product should
do.
Expand Down
2 changes: 1 addition & 1 deletion test/functional/test_site/expected/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ <h2 id="feature-list">Feature list<a class="fa fa-anchor" href="#feature-list"><
</div>
<p><strong>Modal with panel inside</strong></p>
<p><span for="modal-with-panel" v-b-popover.hover.top.html="popoverGenerator" v-b-tooltip.hover.top.html="tooltipContentGetter" v-on:mouseover="$refs['modal-with-panel'].show()" class="trigger">trigger</span></p>
<b-modal id="modal-with-panel" ok-only="" size="" modal-class="mb-zoom" ref="modal-with-panel"><template slot="modal-title">modal title with panel inside</template>
<b-modal id="modal-with-panel" hide-footer="" size="" modal-class="mb-zoom" ref="modal-with-panel"><template slot="modal-title">modal title with panel inside</template>
<panel expanded="" id="panel-inside-modal"><template slot="_header"><h2 id="panel-inside-modal">Panel inside modal<a class="fa fa-anchor" href="#panel-inside-modal"></a></h2></template>
<p><strong>Panel content inside modal</strong></p>
</panel>
Expand Down
2 changes: 1 addition & 1 deletion test/functional/test_site_algolia_plugin/expected/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
<li><a class="dropdown-item" href="/">Two</a></li>
</dropdown>
<p><strong>Modal content should have algolia-no-index class</strong></p>
<b-modal id="modal:trigger_id" ok-only="" size="" modal-class="mb-zoom" ref="modal:trigger_id"><template slot="modal-title">Modal</template> Content should have `algolia-no-index` class
<b-modal id="modal:trigger_id" hide-footer="" size="" modal-class="mb-zoom" ref="modal:trigger_id"><template slot="modal-title">Modal</template> Content should have `algolia-no-index` class
</b-modal>
<span for="modal:trigger_id" v-b-popover.hover.top.html="popoverGenerator" v-b-tooltip.hover.top.html="tooltipContentGetter" v-on:mouseover="$refs['modal:trigger_id'].show()" class="trigger">Trigger should not have `algolia-no-index` class</span>
<p><strong>Panels that are not expanded should have algolia-no-index class</strong></p>
Expand Down
2 changes: 1 addition & 1 deletion test/functional/test_site_templates/test_default/expected/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ <h2 id="sub-heading-1-1">Sub Heading 1.1<a class="fa fa-anchor" href="#sub-headi
<p>A <span effect="scale" placement="top" trigger="hover" data-mb-component-type="tooltip" v-b-tooltip.hover.top.html="tooltipInnerContentGetter" class="trigger"><span data-mb-html-for="_content">❗️ some <strong>important explanation</strong></span>tooltip</span>,
a <span for="modal:modalinfo" trigger="click" v-b-popover.click.top.html="popoverGenerator" v-b-tooltip.click.top.html="tooltipContentGetter" v-on:click="$refs['modal:modalinfo'].show()" class="trigger">modal</span>, a <a href="https://markbind.org/">link</a>,
a <span class="badge badge-danger">badge</span>, another <span class="badge badge-warning">badge</span>.</p>
<b-modal id="modal:modalinfo" ok-only="" size="" modal-class="mb-zoom" ref="modal:modalinfo"><template slot="modal-title">Modal Title</template> Some text some text some text some text some text some text some text. Some text some text some text some text some text some text some text. Some text some text some text some text some text
<b-modal id="modal:modalinfo" hide-footer="" size="" modal-class="mb-zoom" ref="modal:modalinfo"><template slot="modal-title">Modal Title</template> Some text some text some text some text some text some text some text. Some text some text some text some text some text some text some text. Some text some text some text some text some text
some text some text some text some text some text some text some text some text some text. Some text some text some text some text some text some text. Some text some text some text some text some text some text some text.
</b-modal>
<p><strong>A table:</strong></p>
Expand Down
4 changes: 4 additions & 0 deletions test/unit/parsers/componentParser.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ test('parseComponent parses modal attributes and inserts into dom as slots corre
// todo remove these once 'modal-header' / 'modal-footer' for modal is fully deprecated
parseAndVerifyTemplate(testData.PARSE_MODAL_SLOTS_RENAMING,
testData.PARSE_MODAL_SLOTS_RENAMING_EXPECTED);

// when the ok-text attr is set, footer shouldn't be disabled and ok-only attr should be added
parseAndVerifyTemplate(testData.PARSE_MODAL_OK_TEXT,
testData.PARSE_MODAL_OK_TEXT_EXPECTED);
});

test('parseComponent parses tab & tab-group attributes and inserts into dom as slots correctly', () => {
Expand Down
20 changes: 16 additions & 4 deletions test/unit/utils/componentParserData.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ module.exports.PARSE_MODAL_HEADER = `
`;

module.exports.PARSE_MODAL_HEADER_EXPECTED = `
<b-modal ok-only="" size="" modal-class="mb-zoom"><template slot="modal-title"><em>Lorem ipsum dolor sit amet</em></template>
<b-modal hide-footer="" size="" modal-class="mb-zoom"><template slot="modal-title"><em>Lorem ipsum dolor sit amet</em></template>
Header attribute should be inserted as bootstrap-vue modal-title slot.
</b-modal>
`;
Expand All @@ -173,7 +173,7 @@ module.exports.PARSE_MODAL_TITLE = `
`;

module.exports.PARSE_MODAL_TITLE_EXPECTED = `
<b-modal ok-only="" size="" modal-class="mb-zoom"><template slot="modal-title"><strong>Lorem ipsum dolor sit amet</strong></template>
<b-modal hide-footer="" size="" modal-class="mb-zoom"><template slot="modal-title"><strong>Lorem ipsum dolor sit amet</strong></template>
Title attribute should be inserted as internal _header slot.
</b-modal>
`;
Expand All @@ -185,11 +185,23 @@ module.exports.PARSE_MODAL_TITLE_NO_OVERRIDE = `
`;

module.exports.PARSE_MODAL_TITLE_NO_OVERRIDE_EXPECTED = `
<b-modal ok-only="" size="" modal-class="mb-zoom"><template slot="modal-title"><strong>Header header</strong></template>
<b-modal hide-footer="" size="" modal-class="mb-zoom"><template slot="modal-title"><strong>Header header</strong></template>
Title attribute should not have priority over newer header attribute, and should be deleted.
</b-modal>
`;

module.exports.PARSE_MODAL_OK_TEXT = `
<modal ok-text="Custom OK" title="**Title header**" header="**Header header**">
ok-only attr should be set, hide-footer should not be set.
</modal>
`;

module.exports.PARSE_MODAL_OK_TEXT_EXPECTED = `
<b-modal ok-title="Custom OK" ok-only="" size="" modal-class="mb-zoom"><template slot="modal-title"><strong>Header header</strong></template>
ok-only attr should be set, hide-footer should not be set.
</b-modal>
`;

// todo remove these once modal-header modal-footer slot names are deprecated fully.

module.exports.PARSE_MODAL_SLOTS_RENAMING = `
Expand All @@ -200,7 +212,7 @@ module.exports.PARSE_MODAL_SLOTS_RENAMING = `
`;

module.exports.PARSE_MODAL_SLOTS_RENAMING_EXPECTED = `
<b-modal ok-only="" size="" modal-class="mb-zoom">
<b-modal size="" modal-class="mb-zoom">
<div slot="modal-header">Should be renamed to header</div>
<div slot="modal-footer">Should be renamed to footer</div>
</b-modal>
Expand Down