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

Polish messageBox.js #10835

Merged
merged 1 commit into from
Sep 11, 2017
Merged

Polish messageBox.js #10835

merged 1 commit into from
Sep 11, 2017

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Sep 7, 2017

Add white-space: pre-wrap to fix #10119
Remove redundant styles

Auditors: @cezaraugusto

Test Plan:

  1. Open https://jsfiddle.net/9o5tmgst/1/
  2. Make sure the lines are wrapped
  3. Open https://jsfiddle.net/c5o7arqu/
  4. Make sure everything is properly alinged

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

Add white-space: pre-wrap to fix #10119
Remove redundant styles

Auditors: @cezaraugusto

Test Plan:
1. Open https://jsfiddle.net/9o5tmgst/1/
2. Make sure the lines are wrapped
3. Open https://jsfiddle.net/c5o7arqu/
3. Make sure everything is properly alinged
@luixxiul luixxiul added the polish Nice to have — usually related to front-end/visual tasks. label Sep 7, 2017
@luixxiul luixxiul added this to the 0.21.x (Nightly Channel) milestone Sep 7, 2017
@luixxiul luixxiul self-assigned this Sep 7, 2017
@@ -162,34 +161,31 @@ class MessageBox extends React.Component {
}

const styles = StyleSheet.create({
dialog: {
outline: 'none'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dialog has this style thanks to 2c3126f#diff-5d3d90fdb0c3efa6acd09057f2d4b101R85 (messageBox is focused by default).

outline: 'none'
},
container: {
outline: 'none',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

div has outline: none by default.

outline: 'none',
display: 'flex',
flexDirection: 'column',
justifyContent: 'space-between'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

divs are aligned vertically by default so I think those are not necessary.

@codecov-io
Copy link

codecov-io commented Sep 7, 2017

Codecov Report

Merging #10835 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #10835   +/-   ##
=======================================
  Coverage   54.16%   54.16%           
=======================================
  Files         247      247           
  Lines       21548    21548           
  Branches     3338     3338           
=======================================
  Hits        11672    11672           
  Misses       9876     9876
Flag Coverage Δ
#unittest 54.16% <100%> (ø) ⬆️
Impacted Files Coverage Δ
app/renderer/components/common/messageBox.js 87.62% <100%> (ø) ⬆️

Copy link
Contributor

@NejcZdovc NejcZdovc left a comment

Choose a reason for hiding this comment

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

++

@NejcZdovc NejcZdovc merged commit 5b819af into brave:master Sep 11, 2017
@luixxiul luixxiul deleted the polish-messageBox branch September 11, 2017 07:45
@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
polish Nice to have — usually related to front-end/visual tasks.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Line break not recognized in alert window
4 participants