Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Remove smallButton from button.less #9236

Merged
merged 2 commits into from
Jun 9, 2017
Merged

Remove smallButton from button.less #9236

merged 2 commits into from
Jun 9, 2017

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Jun 3, 2017

The classname has only been used on findbar.

Closes #9232
Addresses #9233

Auditors:

Test Plan:

  1. npm run test -- --grep='findBar'
  2. Open the browser
  3. Display find bar
  4. Run manual test plan on https://github.com/brave/browser-laptop/wiki/Manual-Tests#find-on-page
  5. Open about:styles
  6. Make sure the Small Button is not there
  7. Make sure 3 icons are displayed under notificationItem buttons

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.

Test Plan:

Reviewer Checklist:

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

@luixxiul luixxiul added this to the 0.18.x milestone Jun 3, 2017
@luixxiul luixxiul self-assigned this Jun 3, 2017
@luixxiul luixxiul requested a review from cezaraugusto June 3, 2017 15:30
@luixxiul luixxiul removed the request for review from cezaraugusto June 6, 2017 03:25
@luixxiul luixxiul requested a review from cezaraugusto June 6, 2017 03:25
<button data-l10n-id='done' className='browserButton whiteButton smallButton'{'\n'}
onClick={'{this.onRemoveBookmark}'} />
</Code></Pre>

<BrowserButton primaryColor l10nId='primaryColor' onClick={this.onRemoveBookmark} />
<Pre><Code>
&lt;BrowserButton l10nId='cancel' onClick={'{this.onRemoveBookmark}'} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cezaraugusto
Copy link
Contributor

I took advantage of this removal and included iconOnly button, which may benefit downloadBar icons and maybe other icon-only buttons, lmk your thoughts and feel free to tweak

Copy link
Contributor Author

@luixxiul luixxiul left a comment

Choose a reason for hiding this comment

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

9d89df8 LGTM.

Suguru Hirahara and others added 2 commits June 8, 2017 18:27
The classname has only been used on findbar.

Closes #9232
Addresses #9233

Auditors:

Test Plan:
1. npm run test -- --grep='findBar'
2. Open the browser
3. Display find bar
4. Run manual test plan on https://github.com/brave/browser-laptop/wiki/Manual-Tests#find-on-page
@cezaraugusto
Copy link
Contributor

@luixxiul rebased after 40799f9 pls check again. Added some fancy props to iconOnly so we could apply for all icon-buttons we have.

Copy link
Contributor Author

@luixxiul luixxiul left a comment

Choose a reason for hiding this comment

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

++!

@cezaraugusto cezaraugusto merged commit a101f12 into brave:master Jun 9, 2017
@luixxiul luixxiul deleted the remove-smallButton branch June 9, 2017 04:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants