Skip to content
This repository has been archived by the owner on Sep 21, 2023. It is now read-only.

Allow for custom inline styles to be set on the button #483

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

sammce
Copy link

@sammce sammce commented Sep 25, 2021

I was tinkering and discovered that the styles are intended to not be altered.

The change uses Array.forEach() and Object.keys(), both of which are IE 9 compatible.

The test suites pass with the amended snapshot set to reflect the new user defined style prop.

Issue link

Edit:

After forking the repo and using the new inline style capabilities, I came to the conclusion that it might also be useful to be able to customise the style states, to keep a consistent design for all button states.

The new commit allows the following:

Screenshot 2021-09-27 at 19 23 35

Before

Screenshot 2021-09-25 at 17 33 41

After

Screenshot 2021-09-27 at 19 06 49

The implementation of the new style merging is to use a function, named 'addKeysTo' which takes 2 objects, copies a reference to the target object, and assigns each style from the user defined style object too it. The reason that I am using a function for this operation is to follow the DRY principle, and to minimize the bundle size.
This is to keep in line with the general layout of the project, instead of defining the function in google-login.js
@sammce
Copy link
Author

sammce commented Sep 27, 2021

I just tested using the rest operator instead of Object.keys for simplicity's sake, but the bundle size when doing so is 1.2KB larger than doing it the old fashioned way. I imagine this is due to the inclusion of the babel plugin @babel/plugin-proposal-object-rest-spread in the bundle. Just in case you were wondering why I didn't do it that way.

@sammce sammce marked this pull request as draft September 28, 2021 21:47
@sammce sammce closed this Sep 28, 2021
@sammce sammce reopened this Sep 28, 2021
@sammce sammce marked this pull request as ready for review September 28, 2021 22:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants