From 8dd7c0c9842a9c792952efd5718e77301b7c7d4b Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 1 Aug 2017 16:06:51 -0400 Subject: [PATCH] UX: Convert buttons to `d-button` --- .../components/composer-save-button.js.es6 | 7 ++ .../discourse/components/d-button.js.es6 | 4 +- .../discourse/components/share-button.js.es6 | 13 +++ .../discourse/controllers/flag.js.es6 | 9 +- .../discourse/models/composer.js.es6 | 32 +++--- .../components/topic-footer-buttons.hbs | 5 +- .../discourse/templates/composer.hbs | 9 +- .../discourse/templates/modal/flag.hbs | 29 ++++- .../discourse/widgets/button.js.es6 | 4 +- .../discourse/widgets/header.js.es6 | 17 ++- .../discourse/widgets/post-admin-menu.js.es6 | 11 +- .../common/components/buttons.scss | 1 - .../components/d-button-test.js.es6 | 3 +- .../components/share-button-test.js.es6 | 16 +++ test/javascripts/controllers/flag-test.js.es6 | 104 ------------------ 15 files changed, 110 insertions(+), 154 deletions(-) create mode 100644 app/assets/javascripts/discourse/components/composer-save-button.js.es6 create mode 100644 app/assets/javascripts/discourse/components/share-button.js.es6 create mode 100644 test/javascripts/components/share-button-test.js.es6 delete mode 100644 test/javascripts/controllers/flag-test.js.es6 diff --git a/app/assets/javascripts/discourse/components/composer-save-button.js.es6 b/app/assets/javascripts/discourse/components/composer-save-button.js.es6 new file mode 100644 index 00000000000..b33bfb04cc3 --- /dev/null +++ b/app/assets/javascripts/discourse/components/composer-save-button.js.es6 @@ -0,0 +1,7 @@ +import Button from 'discourse/components/d-button'; + +export default Button.extend({ + tabindex: 5, + classNameBindings: [':btn-primary', ':create', 'disableSubmit:disabled'], + title: 'composer.title', +}); diff --git a/app/assets/javascripts/discourse/components/d-button.js.es6 b/app/assets/javascripts/discourse/components/d-button.js.es6 index 565ebdf0ba8..c56e1b6b34a 100644 --- a/app/assets/javascripts/discourse/components/d-button.js.es6 +++ b/app/assets/javascripts/discourse/components/d-button.js.es6 @@ -6,7 +6,7 @@ export default Ember.Component.extend({ tagName: 'button', classNameBindings: [':btn', 'noText', 'btnType'], - attributeBindings: ['disabled', 'translatedTitle:title'], + attributeBindings: ['disabled', 'translatedTitle:title', 'tabindex'], btnIcon: Ember.computed.notEmpty('icon'), @@ -23,7 +23,7 @@ export default Ember.Component.extend({ @computed("title") translatedTitle(title) { - if (title) return I18n.t(title); + return title ? I18n.t(title) : this.get('translatedLabel'); }, @computed("label") diff --git a/app/assets/javascripts/discourse/components/share-button.js.es6 b/app/assets/javascripts/discourse/components/share-button.js.es6 new file mode 100644 index 00000000000..c099e91b507 --- /dev/null +++ b/app/assets/javascripts/discourse/components/share-button.js.es6 @@ -0,0 +1,13 @@ +import Button from 'discourse/components/d-button'; + +export default Button.extend({ + classNames: ['share'], + icon: 'link', + title: 'topic.share.help', + label: 'topic.share.title', + attributeBindings: ['url:data-share-url'], + + click() { + return true; + } +}); diff --git a/app/assets/javascripts/discourse/controllers/flag.js.es6 b/app/assets/javascripts/discourse/controllers/flag.js.es6 index edfa635975e..b8ab2e8845b 100644 --- a/app/assets/javascripts/discourse/controllers/flag.js.es6 +++ b/app/assets/javascripts/discourse/controllers/flag.js.es6 @@ -146,11 +146,10 @@ export default Ember.Controller.extend(ModalFunctionality, { } }.property('selected.name_key', 'userDetails.can_be_deleted', 'userDetails.can_delete_all_posts'), - canSendWarning: function() { - if (this.get("flagTopic")) return false; - - return (Discourse.User.currentProp('staff') && this.get('selected.name_key') === 'notify_user'); - }.property('selected.name_key'), + @computed('flagTopic', 'selected.name_key') + canSendWarning(flagTopic, nameKey) { + return !flagTopic && this.currentUser.get('staff') && nameKey === 'notify_user'; + }, usernameChanged: function() { this.set('userDetails', null); diff --git a/app/assets/javascripts/discourse/models/composer.js.es6 b/app/assets/javascripts/discourse/models/composer.js.es6 index e6f0f0d9d63..48e8e261960 100644 --- a/app/assets/javascripts/discourse/models/composer.js.es6 +++ b/app/assets/javascripts/discourse/models/composer.js.es6 @@ -240,25 +240,25 @@ const Composer = RestModel.extend({ return (this.get('titleLength') <= this.siteSettings.max_topic_title_length); }.property('minimumTitleLength', 'titleLength', 'post.static_doc'), - // The icon for the save button - saveIcon: function () { - switch (this.get('action')) { - case EDIT: return iconHTML('pencil'); - case REPLY: return iconHTML('reply'); - case CREATE_TOPIC: iconHTML('plus'); - case PRIVATE_MESSAGE: iconHTML('envelope'); + @computed('action') + saveIcon(action) { + switch (action) { + case EDIT: return 'pencil'; + case REPLY: return 'reply'; + case CREATE_TOPIC: 'plus'; + case PRIVATE_MESSAGE: 'envelope'; } - }.property('action'), + }, - // The text for the save button - saveText: function() { - switch (this.get('action')) { - case EDIT: return I18n.t('composer.save_edit'); - case REPLY: return I18n.t('composer.reply'); - case CREATE_TOPIC: return I18n.t('composer.create_topic'); - case PRIVATE_MESSAGE: return I18n.t('composer.create_pm'); + @computed('action') + saveLabel(action) { + switch (action) { + case EDIT: return 'composer.save_edit'; + case REPLY: return 'composer.reply'; + case CREATE_TOPIC: return 'composer.create_topic'; + case PRIVATE_MESSAGE: return 'composer.create_pm'; } - }.property('action'), + }, hasMetaData: function() { const metaData = this.get('metaData'); diff --git a/app/assets/javascripts/discourse/templates/components/topic-footer-buttons.hbs b/app/assets/javascripts/discourse/templates/components/topic-footer-buttons.hbs index 15b5882a004..4bd4f094e0d 100644 --- a/app/assets/javascripts/discourse/templates/components/topic-footer-buttons.hbs +++ b/app/assets/javascripts/discourse/templates/components/topic-footer-buttons.hbs @@ -27,10 +27,7 @@ icon="bookmark" action=toggleBookmark}} - + {{share-button url=topic.shareUrl}} {{#if topic.details.can_flag_topic}} {{d-button class="flag-topic" diff --git a/app/assets/javascripts/discourse/templates/composer.hbs b/app/assets/javascripts/discourse/templates/composer.hbs index 9ddc7aeedb5..4127463df84 100644 --- a/app/assets/javascripts/discourse/templates/composer.hbs +++ b/app/assets/javascripts/discourse/templates/composer.hbs @@ -76,7 +76,7 @@ {{popup-input-tip validation=categoryValidation}} {{#if model.archetype.hasOptions}} - + {{d-button action="showOptions" label="topic.options"}} {{/if}} {{/if}} @@ -107,7 +107,12 @@ {{#if canEditTags}} {{tag-chooser tags=model.tags tabIndex="4" categoryId=model.categoryId}} {{/if}} - + {{composer-save-button + action=(action "save") + icon=model.saveIcon + label=model.saveLabel + disableSubmit=disableSubmit}} + {{i18n 'cancel'}} {{#if site.mobileView}} diff --git a/app/assets/javascripts/discourse/templates/modal/flag.hbs b/app/assets/javascripts/discourse/templates/modal/flag.hbs index 465b0a0ed4c..26463f09efb 100644 --- a/app/assets/javascripts/discourse/templates/modal/flag.hbs +++ b/app/assets/javascripts/discourse/templates/modal/flag.hbs @@ -13,17 +13,38 @@ {{/d-modal-body}}
- + {{d-button + class="btn-primary" + action=(action "createFlag") + disabled=submitDisabled + title="flagging.submit_tooltip" + translatedLabel=submitText}} {{#if canSendWarning}} - + {{d-button + class="btn-danger" + action=(action "createFlagAsWarning") + disabled=submitDisabled + icon="exclamation-triangle" + label="flagging.official_warning"}} {{/if}} {{#if canTakeAction}} - + {{d-button + class="btn-danger" + action=(action "takeAction") + disabled=submitDisabled + title="flagging.take_action_tooltip" + icon="gavel" + label="flagging.take_action"}} {{/if}} {{#if canDeleteSpammer}} - + {{d-button + class="btn-danger" + action=(route-action "deleteSpammer" userDetails) + disabled=submitDisabled + icon="exclamation-triangle" + label="flagging.delete_spammer"}} {{/if}}
diff --git a/app/assets/javascripts/discourse/widgets/button.js.es6 b/app/assets/javascripts/discourse/widgets/button.js.es6 index 8d930c50976..ebc7b561495 100644 --- a/app/assets/javascripts/discourse/widgets/button.js.es6 +++ b/app/assets/javascripts/discourse/widgets/button.js.es6 @@ -2,7 +2,7 @@ import { createWidget } from 'discourse/widgets/widget'; import { iconNode } from 'discourse-common/lib/icon-library'; import { h } from 'virtual-dom'; -const ButtonClass = { +export const ButtonClass = { tagName: 'button.widget-button.btn', buildClasses(attrs) { @@ -20,7 +20,7 @@ const ButtonClass = { className += '-text'; } } else if (hasText) { - className += 'btn-text'; + className += ' btn-text'; } return className; diff --git a/app/assets/javascripts/discourse/widgets/header.js.es6 b/app/assets/javascripts/discourse/widgets/header.js.es6 index 600bd753084..6a374e1a8f2 100644 --- a/app/assets/javascripts/discourse/widgets/header.js.es6 +++ b/app/assets/javascripts/discourse/widgets/header.js.es6 @@ -90,11 +90,18 @@ createWidget('header-dropdown', jQuery.extend({ body.push(attrs.contents.call(this)); } - return h('a.icon', { attributes: { href: attrs.href, - 'data-auto-route': true, - title, - 'aria-label': title, - id: attrs.iconId } }, body); + return h( + 'a.icon.btn-flat', + { attributes: { + href: attrs.href, + 'data-auto-route': true, + title, + 'aria-label': title, + id: attrs.iconId + } + }, + body + ); } }, dropdown)); diff --git a/app/assets/javascripts/discourse/widgets/post-admin-menu.js.es6 b/app/assets/javascripts/discourse/widgets/post-admin-menu.js.es6 index 4fb293745a0..379fd4faa5c 100644 --- a/app/assets/javascripts/discourse/widgets/post-admin-menu.js.es6 +++ b/app/assets/javascripts/discourse/widgets/post-admin-menu.js.es6 @@ -1,20 +1,15 @@ import { iconNode } from 'discourse-common/lib/icon-library'; import { createWidget } from 'discourse/widgets/widget'; import { h } from 'virtual-dom'; +import { ButtonClass } from 'discourse/widgets/button'; -createWidget('post-admin-menu-button', { +createWidget('post-admin-menu-button', jQuery.extend(ButtonClass, { tagName: 'li.btn', - buildClasses(attrs) { - return attrs.className; - }, - html(attrs) { - return [iconNode(attrs.icon), I18n.t(attrs.label)]; - }, click() { this.sendWidgetAction('closeAdminMenu'); return this.sendWidgetAction(this.attrs.action); } -}); +})); export default createWidget('post-admin-menu', { tagName: 'div.post-admin-menu.popup-menu', diff --git a/app/assets/stylesheets/common/components/buttons.scss b/app/assets/stylesheets/common/components/buttons.scss index f73c5e3ea09..d433b043424 100644 --- a/app/assets/stylesheets/common/components/buttons.scss +++ b/app/assets/stylesheets/common/components/buttons.scss @@ -5,7 +5,6 @@ // Base // -------------------------------------------------- - .btn { display: inline-block; margin: 0; diff --git a/test/javascripts/components/d-button-test.js.es6 b/test/javascripts/components/d-button-test.js.es6 index 1314406b779..d283da4f21c 100644 --- a/test/javascripts/components/d-button-test.js.es6 +++ b/test/javascripts/components/d-button-test.js.es6 @@ -2,11 +2,12 @@ import componentTest from 'helpers/component-test'; moduleForComponent('d-button', {integration: true}); componentTest('icon only button', { - template: '{{d-button icon="plus"}}', + template: '{{d-button icon="plus" tabindex="3"}}', 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'); + assert.equal(this.$('button').attr('tabindex'), "3", 'it has the tabindex'); } }); diff --git a/test/javascripts/components/share-button-test.js.es6 b/test/javascripts/components/share-button-test.js.es6 new file mode 100644 index 00000000000..41d0fa84814 --- /dev/null +++ b/test/javascripts/components/share-button-test.js.es6 @@ -0,0 +1,16 @@ +import componentTest from 'helpers/component-test'; +moduleForComponent('share-button', {integration: true}); + +componentTest('share button', { + template: '{{share-button url="https://eviltrout.com"}}', + + test(assert) { + assert.ok(this.$(`button.share`).length, 'it has all the classes'); + + assert.ok( + this.$(`button[data-share-url="https://eviltrout.com"]`).length, + 'it has the data attribute for sharing' + ); + } +}); + diff --git a/test/javascripts/controllers/flag-test.js.es6 b/test/javascripts/controllers/flag-test.js.es6 deleted file mode 100644 index 0d4b49e16b3..00000000000 --- a/test/javascripts/controllers/flag-test.js.es6 +++ /dev/null @@ -1,104 +0,0 @@ -import createStore from 'helpers/create-store'; -import AdminUser from 'admin/models/admin-user'; -import { mapRoutes } from 'discourse/mapping-router'; - -var buildPost = function(args) { - return Discourse.Post.create(_.merge({ - id: 1, - can_delete: true, - version: 1 - }, args || {})); -}; - -var buildAdminUser = function(args) { - return AdminUser.create(_.merge({ - id: 11, - username: 'urist' - }, args || {})); -}; - -moduleFor("controller:flag", "controller:flag", { - beforeEach() { - this.registry.register('router:main', mapRoutes()); - }, - needs: ['controller:modal'] -}); - -QUnit.test("canDeleteSpammer not staff", function(assert) { - const store = createStore(); - - var flagController = this.subject({ model: buildPost() }); - sandbox.stub(Discourse.User, 'currentProp').withArgs('staff').returns(false); - - const spamFlag = store.createRecord('post-action-type', {name_key: 'spam'}); - flagController.set('selected', spamFlag); - assert.equal(flagController.get('canDeleteSpammer'), false, 'false if current user is not staff'); -}); - -var canDeleteSpammer = function(assert, flagController, postActionType, expected, testName) { - const store = createStore(); - const flag = store.createRecord('post-action-type', {name_key: postActionType}); - flagController.set('selected', flag); - - assert.equal(flagController.get('canDeleteSpammer'), expected, testName); -}; - -QUnit.test("canDeleteSpammer spam not selected", function(assert) { - sandbox.stub(Discourse.User, 'currentProp').withArgs('staff').returns(true); - var flagController = this.subject({ model: buildPost() }); - flagController.set('userDetails', buildAdminUser({can_delete_all_posts: true, can_be_deleted: true})); - canDeleteSpammer(assert, flagController, 'off_topic', false, 'false if current user is staff, but selected is off_topic'); - canDeleteSpammer(assert, flagController, 'inappropriate', false, 'false if current user is staff, but selected is inappropriate'); - canDeleteSpammer(assert, flagController, 'notify_user', false, 'false if current user is staff, but selected is notify_user'); - canDeleteSpammer(assert, flagController, 'notify_moderators', false, 'false if current user is staff, but selected is notify_moderators'); -}); - -QUnit.test("canDeleteSpammer spam selected", function(assert) { - sandbox.stub(Discourse.User, 'currentProp').withArgs('staff').returns(true); - var flagController = this.subject({ model: buildPost() }); - flagController.set('userDetails', buildAdminUser({can_delete_all_posts: true, can_be_deleted: true})); - canDeleteSpammer(assert, flagController, 'spam', true, 'true if current user is staff, selected is spam, posts and user can be deleted'); - - flagController.set('userDetails', buildAdminUser({can_delete_all_posts: false, can_be_deleted: true})); - canDeleteSpammer(assert, flagController, 'spam', false, 'false if current user is staff, selected is spam, posts cannot be deleted'); - - flagController.set('userDetails', buildAdminUser({can_delete_all_posts: true, can_be_deleted: false})); - canDeleteSpammer(assert, flagController, 'spam', false, 'false if current user is staff, selected is spam, user cannot be deleted'); - - flagController.set('userDetails', buildAdminUser({can_delete_all_posts: false, can_be_deleted: false})); - canDeleteSpammer(assert, flagController, 'spam', false, 'false if current user is staff, selected is spam, user cannot be deleted'); -}); - -QUnit.test("canSendWarning not staff", function(assert) { - const store = createStore(); - - var flagController = this.subject({ model: buildPost() }); - sandbox.stub(Discourse.User, 'currentProp').withArgs('staff').returns(false); - - const notifyUserFlag = store.createRecord('post-action-type', {name_key: 'notify_user'}); - flagController.set('selected', notifyUserFlag); - assert.equal(flagController.get('canSendWarning'), false, 'false if current user is not staff'); -}); - -var canSendWarning = function(assert, flagController, postActionType, expected, testName) { - const store = createStore(); - const flag = store.createRecord('post-action-type', {name_key: postActionType}); - flagController.set('selected', flag); - - assert.equal(flagController.get('canSendWarning'), expected, testName); -}; - -QUnit.test("canSendWarning notify_user not selected", function(assert) { - sandbox.stub(Discourse.User, 'currentProp').withArgs('staff').returns(true); - var flagController = this.subject({ model: buildPost() }); - canSendWarning(assert, flagController, 'off_topic', false, 'false if current user is staff, but selected is off_topic'); - canSendWarning(assert, flagController, 'inappropriate', false, 'false if current user is staff, but selected is inappropriate'); - canSendWarning(assert, flagController, 'spam', false, 'false if current user is staff, but selected is spam'); - canSendWarning(assert, flagController, 'notify_moderators', false, 'false if current user is staff, but selected is notify_moderators'); -}); - -QUnit.test("canSendWarning notify_user selected", function(assert) { - sandbox.stub(Discourse.User, 'currentProp').withArgs('staff').returns(true); - var flagController = this.subject({ model: buildPost() }); - canSendWarning(assert, flagController, 'notify_user', true, 'true if current user is staff, selected is notify_user'); -});