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

Fix margins on legends #2143

Merged
merged 5 commits into from
Jun 19, 2018
Merged

Fix margins on legends #2143

merged 5 commits into from
Jun 19, 2018

Conversation

jesusbotella
Copy link
Contributor

This PR fixes margins and spacing on legends, specially on Bubble legend.

The problem was that the legend was taking more space than what it has assigned due to absolute positioning. So the margins were collapsed, appearing misaligned.

Acceptance

See https://github.com/CartoDB/support/issues/1566

Related to https://github.com/CartoDB/support/issues/1566

right: 0;
width: 100%;
height: 100px;
margin: 10px 0;
margin: 16px 0 10px 0;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shorthands of length 4 are not allowed. Value was 16px 0 10px 0
Shorthand form for property margin should be written more concisely as 16px 0 10px instead of 16px 0 10px 0

Copy link
Contributor

@rubenmoya rubenmoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, but we have to do a really good acceptance since we've been fixing bugs on legends for the last 2 months...

@@ -230,7 +230,7 @@ $maxLegendContainerHeight: 300px;
.CDB-LayerLegends {
margin-top: 0;

+ .CDB-LayerLegends {
&:not(:empty) + &:not(:empty) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brain-explode

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💥

@arianaescobar
Copy link

The margin error is fixed, but I found some other weird behaviors about the legends but unrelated to the margins. Should I open another issue or share them here?

@jesusbotella
Copy link
Contributor Author

@arianaescobar Can you open a new issue with the things you found, please? :)

@arianaescobar
Copy link

Sure!

@arianaescobar
Copy link

I'm just going to share one of those here because it might be related.

screen shot 2018-06-18 at 15 33 42

In the embed, if I collapse the legends, an extra margin appears on the bottom.

@jesusbotella
Copy link
Contributor Author

Yes, I have just checked the last change of that behaviour and it was 2 months ago. So, I can do the change but seems like it was done on purpose 🤔

@arianaescobar
Copy link

"🤔" indeed

@jesusbotella
Copy link
Contributor Author

I don't get your last comment 😂. Do you want me to change that margin?

@ramiroaznar
Copy link
Contributor

The RT has dealt with lots of bug-legends, look at support or cartodb repos and you will find the culprit.

@arianaescobar
Copy link

@jesusbotella Yes, change it please 🙂 it should be the same margin on each side.

@jesusbotella
Copy link
Contributor Author

@arianaescobar I have just noticed that the margin change needs to be done in Builder, so I am gonna open a new issue for that. I'll link it in here once it is opened.

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.

5 participants