From 75d10a4098f913545cbc96d58a7cc631ec9e978c Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 1 Aug 2017 15:33:31 -0400 Subject: [PATCH] UX: Add consistent classes to buttons --- .../discourse/components/d-button.js.es6 | 13 +++++- .../discourse/widgets/button.js.es6 | 25 ++++++++--- .../discourse/widgets/topic-admin-menu.js.es6 | 2 +- .../discourse/widgets/topic-timeline.js.es6 | 6 +-- .../components/d-button-test.js.es6 | 30 +++++++++++++ test/javascripts/widgets/button-test.js.es6 | 43 +++++++++++++++++++ 6 files changed, 108 insertions(+), 11 deletions(-) create mode 100644 test/javascripts/components/d-button-test.js.es6 create mode 100644 test/javascripts/widgets/button-test.js.es6 diff --git a/app/assets/javascripts/discourse/components/d-button.js.es6 b/app/assets/javascripts/discourse/components/d-button.js.es6 index bd606facafd..565ebdf0ba8 100644 --- a/app/assets/javascripts/discourse/components/d-button.js.es6 +++ b/app/assets/javascripts/discourse/components/d-button.js.es6 @@ -5,9 +5,20 @@ export default Ember.Component.extend({ layoutName: 'components/d-button', tagName: 'button', - classNameBindings: [':btn', 'noText'], + classNameBindings: [':btn', 'noText', 'btnType'], attributeBindings: ['disabled', 'translatedTitle:title'], + btnIcon: Ember.computed.notEmpty('icon'), + + @computed('icon', 'translatedLabel') + btnType(icon, translatedLabel) { + if (icon) { + return translatedLabel ? "btn-icon-text" : "btn-icon"; + } else if (translatedLabel) { + return "btn-text"; + } + }, + noText: Ember.computed.empty('translatedLabel'), @computed("title") diff --git a/app/assets/javascripts/discourse/widgets/button.js.es6 b/app/assets/javascripts/discourse/widgets/button.js.es6 index ece396abaa3..8d930c50976 100644 --- a/app/assets/javascripts/discourse/widgets/button.js.es6 +++ b/app/assets/javascripts/discourse/widgets/button.js.es6 @@ -1,15 +1,26 @@ import { createWidget } from 'discourse/widgets/widget'; import { iconNode } from 'discourse-common/lib/icon-library'; - +import { h } from 'virtual-dom'; const ButtonClass = { - tagName: 'button.widget-button', + tagName: 'button.widget-button.btn', buildClasses(attrs) { - const className = this.attrs.className || ''; + let className = this.attrs.className || ''; - if (!attrs.label && !attrs.contents) { - return className + ' no-text'; + let hasText = attrs.label || attrs.contents; + + if (!hasText) { + className += ' no-text'; + } + + if (attrs.icon) { + className += ' btn-icon'; + if (hasText) { + className += '-text'; + } + } else if (hasText) { + className += 'btn-text'; } return className; @@ -39,7 +50,9 @@ const ButtonClass = { const contents = []; const left = !attrs.iconRight; if (attrs.icon && left) { contents.push(iconNode(attrs.icon, { class: attrs.iconClass })); } - if (attrs.label) { contents.push(I18n.t(attrs.label, attrs.labelOptions)); } + if (attrs.label) { + contents.push(h('span.d-button-label', I18n.t(attrs.label, attrs.labelOptions))); + } if (attrs.contents) { contents.push(attrs.contents); } if (attrs.icon && !left) { contents.push(iconNode(attrs.icon, { class: attrs.iconClass })); } diff --git a/app/assets/javascripts/discourse/widgets/topic-admin-menu.js.es6 b/app/assets/javascripts/discourse/widgets/topic-admin-menu.js.es6 index 64ca6395c00..dce68c0af0c 100644 --- a/app/assets/javascripts/discourse/widgets/topic-admin-menu.js.es6 +++ b/app/assets/javascripts/discourse/widgets/topic-admin-menu.js.es6 @@ -32,7 +32,7 @@ createWidget('topic-admin-menu-button', { // We don't show the button when expanded on the right side if (!(attrs.rightSide && state.expanded)) { result.push(this.attach('button', { - className: 'btn toggle-admin-menu' + (attrs.fixed ? " show-topic-admin" : ""), + className: 'toggle-admin-menu' + (attrs.fixed ? " show-topic-admin" : ""), title: 'topic_admin_menu', icon: 'wrench', action: 'showAdminMenu', diff --git a/app/assets/javascripts/discourse/widgets/topic-timeline.js.es6 b/app/assets/javascripts/discourse/widgets/topic-timeline.js.es6 index bccb90f80f4..eab0694e7d3 100644 --- a/app/assets/javascripts/discourse/widgets/topic-timeline.js.es6 +++ b/app/assets/javascripts/discourse/widgets/topic-timeline.js.es6 @@ -15,7 +15,7 @@ function clamp(p, min=0.0, max=1.0) { function attachBackButton(widget) { return widget.attach('button', { - className: 'btn btn-primary btn-small back-button', + className: 'btn-primary btn-small back-button', label: 'topic.timeline.back', title: 'topic.timeline.back_description', action: 'goBack' @@ -371,7 +371,7 @@ export default createWidget('topic-timeline', { if (currentUser && !attrs.fullScreen) { if (attrs.topic.get('details.can_create_post')) { controls.push(this.attach('button', { - className: 'btn create', + className: 'create', icon: 'reply', title: 'topic.reply.help', action: 'replyToPost' @@ -382,7 +382,7 @@ export default createWidget('topic-timeline', { if (attrs.fullScreen) { controls.push(this.attach('button', { - className: 'btn jump-to-post', + className: 'jump-to-post', title: 'topic.progress.jump_prompt_long', label: 'topic.progress.jump_prompt', action: 'jumpToPostPrompt' diff --git a/test/javascripts/components/d-button-test.js.es6 b/test/javascripts/components/d-button-test.js.es6 new file mode 100644 index 00000000000..1314406b779 --- /dev/null +++ b/test/javascripts/components/d-button-test.js.es6 @@ -0,0 +1,30 @@ +import componentTest from 'helpers/component-test'; +moduleForComponent('d-button', {integration: true}); + +componentTest('icon only button', { + template: '{{d-button icon="plus"}}', + + test(assert) { + assert.ok(this.$('button.btn.btn-icon.no-text').length, 'it has all the classes'); + assert.ok(this.$('button .d-icon.d-icon-plus').length, 'it has the icon'); + } +}); + +componentTest('icon and text button', { + template: '{{d-button icon="plus" label="topic.create"}}', + + test(assert) { + assert.ok(this.$('button.btn.btn-icon-text').length, 'it has all the classes'); + assert.ok(this.$('button .d-icon.d-icon-plus').length, 'it has the icon'); + assert.ok(this.$('button span.d-button-label').length, 'it has the label'); + } +}); + +componentTest('text only button', { + template: '{{d-button label="topic.create"}}', + + test(assert) { + assert.ok(this.$('button.btn.btn-text').length, 'it has all the classes'); + assert.ok(this.$('button span.d-button-label').length, 'it has the label'); + } +}); diff --git a/test/javascripts/widgets/button-test.js.es6 b/test/javascripts/widgets/button-test.js.es6 new file mode 100644 index 00000000000..96bd4b245c7 --- /dev/null +++ b/test/javascripts/widgets/button-test.js.es6 @@ -0,0 +1,43 @@ +import { moduleForWidget, widgetTest } from 'helpers/widget-test'; + +moduleForWidget('button'); + +widgetTest('icon only button', { + template: '{{mount-widget widget="button" args=args}}', + + beforeEach() { + this.set('args', { icon: 'smile-o' }); + }, + + test(assert) { + assert.ok(this.$('button.btn.btn-icon.no-text').length, 'it has all the classes'); + assert.ok(this.$('button .d-icon.d-icon-smile-o').length, 'it has the icon'); + } +}); + +widgetTest('icon and text button', { + template: '{{mount-widget widget="button" args=args}}', + + beforeEach() { + this.set('args', { icon: 'plus', label: 'topic.create' }); + }, + + test(assert) { + assert.ok(this.$('button.btn.btn-icon-text').length, 'it has all the classes'); + assert.ok(this.$('button .d-icon.d-icon-plus').length, 'it has the icon'); + assert.ok(this.$('button span.d-button-label').length, 'it has the label'); + } +}); + +widgetTest('text only button', { + template: '{{mount-widget widget="button" args=args}}', + + beforeEach() { + this.set('args', { label: 'topic.create' }); + }, + + test(assert) { + assert.ok(this.$('button.btn.btn-text').length, 'it has all the classes'); + assert.ok(this.$('button span.d-button-label').length, 'it has the label'); + } +});