material-design-light icon indicating copy to clipboard operation
material-design-light copied to clipboard

fixed #29

Open stephanebastian opened this issue 2 years ago • 4 comments

This is a WIP. textfield-filled is 90% done. outlined is completely broken

@robinlahtinen : This is the first stab at fixing #29 . It's currently working on Firefox and Chrome. I believe it should be working with fairly old browsers and the default mode is 'populated' so that old browsers don't supporting fancy css would work. Note that I changed classnames to be 'textfield-' instwerd of 'fieldtext-' and such. Whenever you have time I would appreciate any feedback you may have.

stephanebastian avatar Mar 10 '23 18:03 stephanebastian

@robinlahtinen I believe this PR fixes #29. Could you please do a full review whenever you have some spare time ? As a side note, you will see that I've added the file _md.scss and updated the import section. When I got started on implementing this feature, I found that it was somewhat a pain to prefix theme related variables with 'light', or state related variables with state.$varName (same goes for typo and such). So I added the file md.scss. The goal is to import only this file (@use 'md' as *) so we reference variables as $md-sys-typescale-body-large-size. It's not a big deal but I found it easier when doing a cut/paste of variables from the material design spec. Wondering if you´d feel the same. Cheers

stephanebastian avatar Mar 14 '23 13:03 stephanebastian

Not sure why the labeler is failing. Might be related to this issue though: https://github.com/actions/labeler/issues/12 it seems to be a permission issue. Do you have an idea on how to fix it?

stephanebastian avatar Mar 14 '23 18:03 stephanebastian

I found that it was somewhat a pain to prefix theme related variables with 'light' [...] It's not a big deal but I found it easier when doing a cut/paste of variables from the material design spec.

Good idea! Material Design Light supports dark theme, so that should also be taken in to concern.

Could you please do a full review whenever you have some spare time ?

Code looks good and seems to be up to the specifications mostly. It is missing support for dark theme and some lesser problems I referenced directly on the code reviews below.

  • When hovering mouse over the text field, the cursor doesn't change the type to text (shape of I-beam) mode on the inner sides.
  • Text fields are missing some of the status effects, eg. on-hover color changes.

robinlahtinen avatar Mar 15 '23 21:03 robinlahtinen

@robinlahtinen I've made the changes you requested, splitted the file in two files, one for filled text fields and another one for outlined text fields. Could you please review one more time and merge if its ok? Thx

stephanebastian avatar Apr 05 '23 15:04 stephanebastian