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

Cancel the margin-left of browserButton with groupedItem #11290

Merged
merged 1 commit into from
Oct 13, 2017
Merged

Cancel the margin-left of browserButton with groupedItem #11290

merged 1 commit into from
Oct 13, 2017

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Oct 5, 2017

Closes #11288

Auditors: @cezaraugusto

Test Plan:

  1. Open about:styles
  2. Click 'buttons'
  3. Make sure the first button in a group does not have the left margin

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 misc/button polish Nice to have — usually related to front-end/visual tasks. labels Oct 5, 2017
@luixxiul luixxiul self-assigned this Oct 5, 2017
@luixxiul luixxiul added this to the 0.21.x (Nightly Channel) milestone Oct 5, 2017
Closes #11288

Auditors:

Test Plan:
1. Open about:styles
2. Click 'buttons'
3. Make sure the first button in a group does not have the left margin
@codecov-io
Copy link

codecov-io commented Oct 11, 2017

Codecov Report

Merging #11290 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #11290      +/-   ##
==========================================
- Coverage   52.48%   52.45%   -0.04%     
==========================================
  Files         268      268              
  Lines       25229    25229              
  Branches     4026     4026              
==========================================
- Hits        13242    13233       -9     
- Misses      11987    11996       +9
Flag Coverage Δ
#unittest 52.45% <ø> (-0.04%) ⬇️
Impacted Files Coverage Δ
app/renderer/components/common/browserButton.js 69.76% <ø> (ø) ⬆️
js/stores/appStoreRenderer.js 91.17% <0%> (-8.83%) ⬇️
app/renderer/components/reduxComponent.js 84.37% <0%> (-6.25%) ⬇️
js/stores/windowStore.js 27.27% <0%> (-0.31%) ⬇️

Copy link
Contributor

@cezaraugusto cezaraugusto 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 e1fb1d3 into brave:master Oct 13, 2017
@cezaraugusto cezaraugusto deleted the polish-browserButton-groupedItem branch October 13, 2017 18:19
@luixxiul
Copy link
Contributor Author

luixxiul commented Oct 13, 2017

As a follow up I'll add a comment to about:styles#buttons about https://github.com/brave/browser-laptop/pull/11290/files#diff-0ff973318abdc5c07f55cefe86bd5ed7R221 (The buttons have to have a parent. Beginning with Selectors Level 4, that is no longer required, adding a link to the MDN knowledge base's article which I referred)

#11522

@luixxiul
Copy link
Contributor Author

@cezaraugusto would you mind adding this to 0.21.x? cc @bbondy

cezaraugusto added a commit that referenced this pull request Oct 13, 2017
Cancel the margin-left of browserButton with groupedItem
@cezaraugusto
Copy link
Contributor

cezaraugusto commented Oct 13, 2017

0.21.x 9e730c5

@bbondy
Copy link
Member

bbondy commented Oct 23, 2017

confirmed it is already there:
0.21.x: dc237ec
master: 9e730c5

@luixxiul
Copy link
Contributor Author

thanks!

@bbondy bbondy modified the milestones: 0.21.x (Developer Channel), 0.20.x (Beta Channel) Oct 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
misc/button polish Nice to have — usually related to front-end/visual tasks.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants