Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for card_margin in grid layouts #195

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

Conversation

cdhgee
Copy link

@cdhgee cdhgee commented Apr 10, 2022

Fixes #194

@cdhgee cdhgee changed the title Add support for card_param in grid layouts Add support for card_margin in grid layouts Apr 14, 2022
@Madelena
Copy link

Has this been merged yet? This fix is sorely needed.

@M4th1asK
Copy link

M4th1asK commented Sep 1, 2022

@thomasloven

@awgneo
Copy link

awgneo commented Sep 6, 2022

This code looks good :) Need this as well ;) *NVM noticed a bug

@awgneo
Copy link

awgneo commented Sep 6, 2022

I corrected and tested this change in this PR instead https://github.com/thomasloven/lovelace-layout-card/pull/210/files :)

@cdhgee
Copy link
Author

cdhgee commented Sep 6, 2022

Looks like you only changed line 152 in grid.ts, correct?

from

image

to

image

The fallback value of 4px 4px 8px was deliberate so that it retains the existing behavior if not explicitly defined. Your version of the PR would introduce a breaking change.

@awgneo
Copy link

awgneo commented Sep 13, 2022

When I tested this PR, my margins were not appropriately applied. So instead, I mirrored the existing column logic of

margin: var(--card-margin);
in my PR, which already has a fallback defined in its configuration
I supported this strategy in the same way in my PR https://github.com/thomasloven/lovelace-layout-card/pull/210/files#diff-4f9f8ab90608430f9ad38a4a682f3e9f7cbd2aa69d11a636ec18b84ea33f5094R62, and it works as expected without breaking existing logic, with or without setting card_margin.

@pafnow
Copy link

pafnow commented Dec 13, 2022

Has this been merged yet? This fix is sorely needed.

Me too !

@snjoetw
Copy link

snjoetw commented Dec 31, 2022

@thomasloven can we have this merged please?

@EmJay276
Copy link

EmJay276 commented Jan 1, 2023

Isn't this the same with additional overflow? #210

@sciurius
Copy link

sciurius commented Feb 26, 2023

@cdhgee With your version of card-layout.js I still get unwanted margins.

views:
      - type: custom:grid-layout
        layout:
          grid-template-columns: 50% 50%
          margin: 0
          padding: 0
          card_margin: 0 0 0 0
        cards:
              - type: markdown
                content: 'a'
              - type: markdown
                content: 'b'
              - type: markdown
                content: 'c'
              - type: markdown
                content: 'd'

Did I misunderstand this feature, or how to apply? I just copied card-layout.js to www/community/lovelace-layout-card/.

scrot20230226152721

@cdhgee
Copy link
Author

cdhgee commented Feb 26, 2023 via email

@cdhgee
Copy link
Author

cdhgee commented Feb 27, 2023

@sciurius This is how it looks with your code on my system:

image

I've noticed that Lovelace generates/uses gzipped versions of JS files in some instances. I suspect that even though you copied over the card-layout.js file, Lovelace is using the card-layout.js.gz compressed version. Renaming or removing the compressed version and restarting HA should force it to start using the updated card-layout.js file.

image

@sciurius
Copy link

I did replace layout-card.js, layout-card.js.gz, rollup.config.js and rollup.config.js.gz and restarted HA (and the browser) several times... The size difference shown in your screenshot made me realize that I forgot to switch to the bugfix-grid-layout-margin branch first.
Now it is working as expected. Great job!

For the sake of completeness, layout-card.js in the bugfix-grid-layout-margin branch of the repo is 49409 bytes while the one shown is 49535 bytes.

@cdhgee
Copy link
Author

cdhgee commented Feb 27, 2023

@sciurius Glad you got it working. The difference in size in my local copy vs the PR copy of layout-card.js is because I tweaked the console output of my local copy so that I could be certain that my updated copy was being used.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parameter "card_margin" ignored when using grid layout
8 participants