feat(fab): Enable padding customization #2959

abhiomkar · 2018-06-18T23:54:37Z

This change enables padding customization for Extended FAB including icon & label side padding. This approach uses Sass variables instead of mixins since the icons side padding needs to be calculated based on the label side padding. It avoids creating additional class name for Extended FAB without icon.

…dding_issue_2900

codecov-io · 2018-06-19T00:03:30Z

Codecov Report

Merging #2959 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #2959   +/-   ##
======================================
  Coverage    98.3%   98.3%           
======================================
  Files         101     101           
  Lines        4368    4368           
  Branches      564     564           
======================================
  Hits         4294    4294           
  Misses         74      74

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 153e737...852e47f. Read the comment docs.

kfranqueiro · 2018-06-20T20:03:46Z

Discussed with @abhiomkar offline. Instead of using global variables, we'll create mixins, to allow for the potential of different customizations for FABs on different views (e.g. a primary/front view vs. secondary/subsidiary views).

I had initially pondered encoding the relationship between icon size and padding all within the icon-size mixin, but Abhinay pointed out that it would probably require significantly more mental overhead for the simple case where someone has redlines they want to apply. So instead, we'll expose mixins allowing direct adjustment of the icon and label paddings.

Otherwise, the approach in this PR is nice, since the negative margin automatically accommodates the intended difference between label-side and icon-side edge padding.

kfranqueiro · 2018-06-22T16:02:55Z

packages/mdc-fab/_mixins.scss

+}
+
+@mixin mdc-fab-extended-label-padding($label-padding) {
+  padding: 0 $mdc-fab-extended-label-padding;


$label-padding

kfranqueiro · 2018-06-22T16:03:10Z

packages/mdc-fab/_mixins.scss

+  .mdc-fab__icon {
+    @include mdc-rtl-reflexive-property(
+      margin,
+      $mdc-fab-extended-icon-padding - $mdc-fab-extended-label-padding,


The variable names throughout these mixins need to be updated to reference the mixin parameters, otherwise they will never actually customize anything.

We should ideally have an additional screenshot test page under mdc-fab/mixins to test these.

kfranqueiro · 2018-06-22T16:12:25Z

packages/mdc-fab/README.md

@@ -112,6 +112,8 @@ Mixin | Description
 `mdc-fab-container-color($color)` | Sets the container color to the given color
 `mdc-fab-icon-size($width, $height)` | Sets the icon `width`, `height`, and `font-size` properties to the specified `width` and `height`. `$height` is optional and will default to `$width` if omitted. The `font-size` will be set to the provided `$width` value.
 `mdc-fab-ink-color($color)` | Sets the ink color to the given color
+`mdc-fab-extended-padding($icon-padding, $label-padding)` | Sets the padding of icon sides (Left side and gutter space) and label side for Extended FAB. The left & right padding of the icon are equal.


I'm suggesting some rephrasings for each of these, to try to simplify and clarify when to use each.

"Sets the padding on both sides of the icon, and between the label and the edge of the FAB. In cases where there is no icon, $label-padding will apply to both sides."

kfranqueiro · 2018-06-22T16:16:01Z

packages/mdc-fab/README.md

@@ -112,6 +112,8 @@ Mixin | Description
 `mdc-fab-container-color($color)` | Sets the container color to the given color
 `mdc-fab-icon-size($width, $height)` | Sets the icon `width`, `height`, and `font-size` properties to the specified `width` and `height`. `$height` is optional and will default to `$width` if omitted. The `font-size` will be set to the provided `$width` value.
 `mdc-fab-ink-color($color)` | Sets the ink color to the given color
+`mdc-fab-extended-padding($icon-padding, $label-padding)` | Sets the padding of icon sides (Left side and gutter space) and label side for Extended FAB. The left & right padding of the icon are equal.
+`mdc-fab-extended-label-padding($label-padding)` | Sets the label side padding for Extended FAB. This padding size will be applied to both sides when Extended FAB is rendered without icon.


Change the second sentence to "Useful in lieu of mdc-fab-extended-padding when styling an Extended FAB with no icon."

…to README." Also added new screenshot tests for mixins

kfranqueiro · 2018-06-22T17:54:10Z

packages/mdc-fab/README.md

@@ -112,8 +112,8 @@ Mixin | Description
 `mdc-fab-container-color($color)` | Sets the container color to the given color
 `mdc-fab-icon-size($width, $height)` | Sets the icon `width`, `height`, and `font-size` properties to the specified `width` and `height`. `$height` is optional and will default to `$width` if omitted. The `font-size` will be set to the provided `$width` value.
 `mdc-fab-ink-color($color)` | Sets the ink color to the given color
-`mdc-fab-extended-padding($icon-padding, $label-padding)` | Sets the padding of icon sides (Left side and gutter space) and label side for Extended FAB. The left & right padding of the icon are equal.
-`mdc-fab-extended-label-padding($label-padding)` | Sets the label side padding for Extended FAB. This padding size will be applied to both sides when Extended FAB is rendered without icon.
+`mdc-fab-extended-padding($icon-padding, $label-padding)` | Sets the padding on both sides of the icon, and between the label and the edge of the FAB. In cases where there is no icon, $label-padding will apply to both sides.


Nit: backticks around $label-padding

kfranqueiro · 2018-06-22T17:54:43Z

test/screenshot/golden.json

@@ -125,6 +125,15 @@
      "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/kfranqueiro/2018/05/25/21_14_58_299/6b0f3e00/mdc-fab/classes/mini.html.win10_ie11.png"
    }
  },
+  "mdc-fab/mixins/mdc-fab-extended-label-adding.html": {


Typo in filename? adding -> padding?

…dding_issue_2900

kfranqueiro · 2018-06-22T19:32:01Z

packages/mdc-fab/package.json

@@ -18,6 +18,7 @@
    "@material/elevation": "^0.36.1",
    "@material/ripple": "^0.36.0",
    "@material/theme": "^0.35.0",
-    "@material/typography": "^0.35.0"
+    "@material/typography": "^0.35.0",
+    "@material/rtl": "^0.36.0"


Nit: put rtl after ripple

abhiomkar added 4 commits June 18, 2018 19:21

fix(fab): Added icon / label padding variables for customizations.

2afd27c

feat(fab): Updated golden screenshots and README

785ba54

feat(fab): Fixed typos and lint errors

38a2421

Merge remote-tracking branch 'origin/master' into feat_extendedfab_pa…

1c710f4

…dding_issue_2900

abhiomkar requested a review from kfranqueiro June 18, 2018 23:55

abhiomkar added 2 commits June 19, 2018 15:23

feat(fab): Extended FAB - Added support for RTL

dbbb4c9

feat(fab): Extended FAB - Added support for text label followed by icon

a6fbf27

abhiomkar added 2 commits June 20, 2018 17:15

feat(fab): Added 2 new mixins to customize icon & text label padding.

2f51c65

feat(fab): minor correction to sass mixin README

2cf975b

kfranqueiro suggested changes Jun 22, 2018

View reviewed changes

abhiomkar added 2 commits June 22, 2018 13:08

feat(fab): Corrected mixins to use local variables and minor changes …

ffff6b0

…to README." Also added new screenshot tests for mixins

feat(fab): Updated golden for fab mixins

ba194d9

kfranqueiro suggested changes Jun 22, 2018

View reviewed changes

abhiomkar added 3 commits June 22, 2018 14:39

Merge remote-tracking branch 'origin/master' into feat_extendedfab_pa…

bef7a63

…dding_issue_2900

feat(fab): rename label-adding.html => label-padding.html

c068d6e

feat(fab): Updated goldens

e1e8e81

kfranqueiro approved these changes Jun 22, 2018

View reviewed changes

feat(fab): Reorder fab's package.json

852e47f

abhiomkar merged commit 1f5fd1f into master Jun 22, 2018

kfranqueiro deleted the feat_extendedfab_padding_issue_2900 branch August 1, 2018 16:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(fab): Enable padding customization #2959

feat(fab): Enable padding customization #2959

abhiomkar commented Jun 18, 2018

codecov-io commented Jun 19, 2018 •

edited

Loading

kfranqueiro commented Jun 20, 2018

kfranqueiro Jun 22, 2018

kfranqueiro Jun 22, 2018

kfranqueiro Jun 22, 2018

kfranqueiro Jun 22, 2018

kfranqueiro Jun 22, 2018

kfranqueiro Jun 22, 2018

kfranqueiro Jun 22, 2018

feat(fab): Enable padding customization #2959

feat(fab): Enable padding customization #2959

Conversation

abhiomkar commented Jun 18, 2018

codecov-io commented Jun 19, 2018 • edited Loading

Codecov Report

kfranqueiro commented Jun 20, 2018

kfranqueiro Jun 22, 2018

Choose a reason for hiding this comment

kfranqueiro Jun 22, 2018

Choose a reason for hiding this comment

kfranqueiro Jun 22, 2018

Choose a reason for hiding this comment

kfranqueiro Jun 22, 2018

Choose a reason for hiding this comment

kfranqueiro Jun 22, 2018

Choose a reason for hiding this comment

kfranqueiro Jun 22, 2018

Choose a reason for hiding this comment

kfranqueiro Jun 22, 2018

Choose a reason for hiding this comment

codecov-io commented Jun 19, 2018 •

edited

Loading