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

Increase top/bottom padding #2721

Merged
merged 7 commits into from
Mar 11, 2015
Merged

Conversation

viddo
Copy link
Contributor

@viddo viddo commented Mar 11, 2015

Fixes #2654

screen shot 2015-03-11 at 13 36 59

compare with previous: screen shot 2015-03-06 at 16 20 51

@viddo
Copy link
Contributor Author

viddo commented Mar 11, 2015

@xavijam review the CSS please, AFAIK we don't have any modals that should need scrolling on x-axis, right?

@viddo
Copy link
Contributor Author

viddo commented Mar 11, 2015

cc @saleiva for reported issue

@xavijam
Copy link
Contributor

xavijam commented Mar 11, 2015

Have you checked it in the create modal window?

@viddo
Copy link
Contributor Author

viddo commented Mar 11, 2015

@xavijam Noticed that the scrolling issue also happend on the create dialog now when double-checking, fixed in 48d66b5 Apart from that, anything else I should check for?

@xavijam
Copy link
Contributor

xavijam commented Mar 11, 2015

Nope, AFAIK, there isn't any modal with x scroll, so 👍

@@ -23,7 +23,7 @@ body.is-inDialog {
background: -ms-linear-gradient(top, rgba(white,1) 0%, rgba(white,0.95) 50%, rgba(white,0.9) 100%);
background: linear-gradient(to bottom, rgba(white,1) 0%, rgba(white,0.95) 50%, rgba(white,0.9) 100%);
filter: progid:DXImageTransform.Microsoft.gradient( startColorstr='#ffffff', endColorstr='#ffffff', GradientType=1 );
overflow: scroll;
overflow-y: scroll;
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, why we set scroll instead of auto?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're right, just noticed it too.. I remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't know if there is any good reason for it, and it's at least not documented here. :/

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd set as auto, x and y, wouldn't you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking in the VCS history I found this: 3e33603

But from what I can see now it should not be necessary anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Auto is the default so think removing is fine. :)

On Wed, Mar 11, 2015 at 2:36 PM, Javier Álvarez Medina <
notifications@github.com> wrote:

In app/assets/stylesheets/new_common/dialog.css.scss
#2721 (comment):

@@ -23,7 +23,7 @@ body.is-inDialog {
background: -ms-linear-gradient(top, rgba(white,1) 0%, rgba(white,0.95) 50%, rgba(white,0.9) 100%);
background: linear-gradient(to bottom, rgba(white,1) 0%, rgba(white,0.95) 50%, rgba(white,0.9) 100%);
filter: progid:DXImageTransform.Microsoft.gradient( startColorstr='#ffffff', endColorstr='#ffffff', GradientType=1 );

  • overflow: scroll;
  • overflow-y: scroll;

I'd set as auto, x and y, wouldn't you?


Reply to this email directly or view it on GitHub
https://github.com/CartoDB/cartodb/pull/2721/files#r26211653.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough.

@xavijam
Copy link
Contributor

xavijam commented Mar 11, 2015

👍

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

viddo added a commit that referenced this pull request Mar 11, 2015
…-maps-styles

Increase top/bottom padding
@viddo viddo merged commit 9738b8e into master Mar 11, 2015
@viddo viddo deleted the 2654-fix-delete-dialog-affected-maps-styles branch March 11, 2015 15:33
@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

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.

Layout errors on the delete dataset modal window
3 participants