chore(button): primary & secondary styles #402

mhuggins · 2018-08-05T13:59:17Z

Description

Adds a PrimaryButton component, which reflects the existing button styles.
Updates the Button component to represent secondary actions.
Smooths the primary button background color to match designs
Uses React.forwardRef to forward refs to the raw DOM element, which prevents us from having to manually implement component functions like focus and blur.

Motivation and Context

Designs have primary and secondary buttons, this brings the styles inline with design.

How Has This Been Tested?

Smoke testing.

Screenshots (if appropriate)

Types of changes

Chore (tests, refactors, and fixes)
New feature (adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

I have read the CONTRIBUTING guidelines and confirm that my code follows the code style of this project.
Tests for the changes have been added (for bug fixes/features)

Documentation

Docs need to be added/updated (for bug fixes/features)

Closing issues

N/A

codecov · 2018-08-05T14:00:14Z

Codecov Report

Merging #402 into develop will increase coverage by 0.08%.
The diff coverage is 66.66%.

@@             Coverage Diff             @@
##           develop     #402      +/-   ##
===========================================
+ Coverage    50.54%   50.62%   +0.08%     
===========================================
  Files          147      148       +1     
  Lines         1189     1195       +6     
  Branches       153      153              
===========================================
+ Hits           601      605       +4     
- Misses         494      496       +2     
  Partials        94       94

Impacted Files	Coverage Δ
...nt/components/TransactionsPanel/Receive/Receive.js	`100% <ø> (ø)`	⬆️
.../account/components/TransactionsPanel/Send/Send.js	`42.1% <ø> (ø)`	⬆️
...erer/login/components/LoginFormWIF/LoginFormWIF.js	`0% <ø> (ø)`	⬆️
...ponents/LoginFormPassphrase/LoginFormPassphrase.js	`0% <ø> (ø)`	⬆️
...r/register/components/RegisterForm/RegisterForm.js	`0% <ø> (ø)`	⬆️
...ponents/LoginFormWalletFile/LoginFormWalletFile.js	`0% <ø> (ø)`	⬆️
...mponents/AccountDetails/SaveAccount/SaveAccount.js	`5.88% <ø> (ø)`	⬆️
...ogin/components/LoginFormLedger/LoginFormLedger.js	`15% <ø> (ø)`	⬆️
...ed/components/Forms/PrimaryButton/PrimaryButton.js	`66.66% <66.66%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 584371f...ecb4f1b. Read the comment docs.

DalderupMaurice · 2018-08-05T14:10:08Z

src/renderer/shared/components/Confirm/Confirm.js

-            </Button>
-            <Button className={classNames(styles.action, styles.cancel)} onClick={onCancel}>
+            </PrimaryButton>
+            <Button


maybe rename SecondaryButton? It's not very clear which style is applied from the name

There are places where we only display a single button and it's not secondary to anything, which is why I left it as just Button. It's only a secondary button when there's not a primary button visible, otherwise it's just the default button style.

mhuggins · 2018-08-05T15:02:12Z

Looks like forwardRef is not supported by enzyme yet: enzymejs/enzyme#1604 and enzymejs/enzyme#1592. I'll take that commit out of this PR and put it into a separate one.

mhuggins · 2018-08-05T15:03:27Z

Updated to remove that last forwardRef commit.

DalderupMaurice · 2018-08-05T15:15:38Z

Hmm still seems to fail 🤔

mhuggins · 2018-08-05T15:17:07Z

src/renderer/shared/components/Forms/Button/Button.js

 };
+
+export default React.forwardRef((props, ref) => {


Oops, I must have included this in the wrong commit. I'll get it fixed.

DalderupMaurice · 2018-08-05T15:40:15Z

Seem to be a conflict after merging the stylelint thing 😅
This piece of code could be left out right? In this case you can resolve it through GH Itself :P

mhuggins · 2018-08-05T15:44:23Z

Rebased, should be good soon!

scottudont · 2018-08-05T19:50:54Z

I know On Aug 5, 2018 8:59 AM, Matt Huggins <notifications@github.com> wrote: Description Adds a PrimaryButton component, which reflects the existing button styles. Updates the Button component to represent secondary actions. Motivation and Context Designs have primary and secondary buttons, this brings the styles inline with design. How Has This Been Tested? Smoke testing. Screenshots (if appropriate) [screen shot 2018-08-05 at 8 56 33 am]<https://user-images.githubusercontent.com/169093/43686572-c70ee6b0-988d-11e8-8d7c-3dda609f0eac.png> Types of changes * Chore (tests, refactors, and fixes) * New feature (adds functionality) * Breaking change (fix or feature that would cause existing functionality to not work as expected) Checklist * I have read the CONTRIBUTING<./CONTRIBUTING.md> guidelines and confirm that my code follows the code style of this project. * Tests for the changes have been added (for bug fixes/features) Documentation * Docs need to be added/updated (for bug fixes/features) Closing issues N/A

…

________________________________ You can view, comment on, or merge this pull request online at: #402 Commit Summary * chore(button): smooth background to match designs * chore(button): differentiate primary and secondary File Changes * M src/renderer/account/components/TransactionsPanel/Receive/Receive.js<https://github.com/nos/client/pull/402/files#diff-0> (4) * M src/renderer/account/components/TransactionsPanel/Send/Send.js<https://github.com/nos/client/pull/402/files#diff-1> (6) * M src/renderer/login/components/LoginFormLedger/LoginFormLedger.js<https://github.com/nos/client/pull/402/files#diff-2> (4) * M src/renderer/login/components/LoginFormPassphrase/LoginFormPassphrase.js<https://github.com/nos/client/pull/402/files#diff-3> (6) * M src/renderer/login/components/LoginFormWIF/LoginFormWIF.js<https://github.com/nos/client/pull/402/files#diff-4> (4) * M src/renderer/login/components/LoginFormWalletFile/LoginFormWalletFile.js<https://github.com/nos/client/pull/402/files#diff-5> (3) * M src/renderer/register/components/AccountDetails/SaveAccount/SaveAccount.js<https://github.com/nos/client/pull/402/files#diff-6> (10) * M src/renderer/register/components/RegisterForm/RegisterForm.js<https://github.com/nos/client/pull/402/files#diff-7> (6) * M src/renderer/settings/components/NetworkSettings/NetworkSettings.js<https://github.com/nos/client/pull/402/files#diff-8> (5) * M src/renderer/shared/components/Alert/Alert.js<https://github.com/nos/client/pull/402/files#diff-9> (10) * M src/renderer/shared/components/Confirm/Confirm.js<https://github.com/nos/client/pull/402/files#diff-10> (14) * M src/renderer/shared/components/Confirm/Confirm.scss<https://github.com/nos/client/pull/402/files#diff-11> (20) * M src/renderer/shared/components/Forms/Button/Button.scss<https://github.com/nos/client/pull/402/files#diff-12> (13) * A src/renderer/shared/components/Forms/PrimaryButton/PrimaryButton.js<https://github.com/nos/client/pull/402/files#diff-13> (38) * A src/renderer/shared/components/Forms/PrimaryButton/PrimaryButton.scss<https://github.com/nos/client/pull/402/files#diff-14> (12) * A src/renderer/shared/components/Forms/PrimaryButton/index.js<https://github.com/nos/client/pull/402/files#diff-15> (1) Patch Links: * https://github.com/nos/client/pull/402.patch * https://github.com/nos/client/pull/402.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#402>, or mute the thread<https://github.com/notifications/unsubscribe-auth/Ali09o7URpxdaQ_8A4C9uo3OTC10HXXbks5uNvo3gaJpZM4VvY4o>.

mhuggins added the PR: needs review Pull request label Aug 5, 2018

mhuggins requested review from DalderupMaurice and jeroenptrs August 5, 2018 13:59

mhuggins changed the title ~~Chore/smooth button~~ chore(button): primary & secondary buttons Aug 5, 2018

mhuggins changed the title ~~chore(button): primary & secondary buttons~~ chore(button): primary & secondary styles Aug 5, 2018

DalderupMaurice reviewed Aug 5, 2018

View reviewed changes

DalderupMaurice added PR: reviewed w/ comments Reviewed but needs to be looked at and removed PR: needs review Pull request labels Aug 5, 2018

mhuggins force-pushed the chore/smooth-button branch from dee8b79 to 1f97f5a Compare August 5, 2018 15:03

mhuggins added PR: needs review Pull request and removed PR: reviewed w/ comments Reviewed but needs to be looked at labels Aug 5, 2018

mhuggins commented Aug 5, 2018

View reviewed changes

mhuggins force-pushed the chore/smooth-button branch from 1f97f5a to 4ef569a Compare August 5, 2018 15:19

DalderupMaurice approved these changes Aug 5, 2018

View reviewed changes

DalderupMaurice added PR: good to merge Reviewed and approved and removed PR: needs review Pull request labels Aug 5, 2018

mhuggins added 2 commits August 5, 2018 10:43

chore(button): smooth background to match designs

fa6b4a8

chore(button): differentiate primary and secondary

ecb4f1b

mhuggins force-pushed the chore/smooth-button branch from 4ef569a to ecb4f1b Compare August 5, 2018 15:43

mhuggins merged commit 6c6e385 into develop Aug 5, 2018

mhuggins deleted the chore/smooth-button branch August 5, 2018 15:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(button): primary & secondary styles #402

chore(button): primary & secondary styles #402

mhuggins commented Aug 5, 2018 •

edited

Loading

codecov bot commented Aug 5, 2018 •

edited

Loading

DalderupMaurice Aug 5, 2018

mhuggins Aug 5, 2018 •

edited

Loading

mhuggins commented Aug 5, 2018 •

edited

Loading

mhuggins commented Aug 5, 2018

DalderupMaurice commented Aug 5, 2018

mhuggins Aug 5, 2018

DalderupMaurice commented Aug 5, 2018

mhuggins commented Aug 5, 2018

scottudont commented Aug 5, 2018 via email

chore(button): primary & secondary styles #402

chore(button): primary & secondary styles #402

Conversation

mhuggins commented Aug 5, 2018 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate)

Types of changes

Checklist

Documentation

Closing issues

codecov bot commented Aug 5, 2018 • edited Loading

Codecov Report

DalderupMaurice Aug 5, 2018

Choose a reason for hiding this comment

mhuggins Aug 5, 2018 • edited Loading

Choose a reason for hiding this comment

mhuggins commented Aug 5, 2018 • edited Loading

mhuggins commented Aug 5, 2018

DalderupMaurice commented Aug 5, 2018

mhuggins Aug 5, 2018

Choose a reason for hiding this comment

DalderupMaurice commented Aug 5, 2018

mhuggins commented Aug 5, 2018

scottudont commented Aug 5, 2018 via email

mhuggins commented Aug 5, 2018 •

edited

Loading

codecov bot commented Aug 5, 2018 •

edited

Loading

mhuggins Aug 5, 2018 •

edited

Loading

mhuggins commented Aug 5, 2018 •

edited

Loading