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

removing horizontal-only padding from the deck-container (themes) #144

Closed

Conversation

twitwi
Copy link
Contributor

@twitwi twitwi commented Nov 9, 2013

There was a padding (horziontal only) for the .deck-container in the reset css. I feel container should have have this padding by default. It also "breaks" the fit extension.

NB: the "make" changes more (e.g. #ccc -> #cccccc) so I commited only the relevant changes.

@imakewebthings
Copy link
Owner

I'm not sure how the padding broke fit. It wasn't changed from the old version, I just moved where it lives.

I think this padding line does need to change though. I'm going to make that change right now and we can look at it again. There was a gross coupling between core, style, and transition themes. Some of that coupling is still present in the code. I do want the padding, so here's what I'm going to do:

  • Clean up the transition themes to not alter the padding for slides.
  • Move the padding from the deck container to top level slides. (For now this will still be using the > .slide convention. I know we talked about dropping that, but its post 1.0 stuff)
  • I think adjust menu's CSS in some way to deal with the above.

I'll close the pull but we can keep discussing here.

@twitwi
Copy link
Contributor Author

twitwi commented Nov 10, 2013

It's true that it is not new (just that I used to have a patched version of the default styles).
I saw some others 48px in the styles indeed, but I did only change the one that was bothering me for my demo presentations.

What you propose (moving the padding into the toplevel slide) is what I would do (and did in my patched version). It plays much better with the fit extension, so it's all good for me.

Thanks.

imakewebthings added a commit that referenced this pull request Nov 10, 2013
@imakewebthings
Copy link
Owner

@twitwi Hopefully the latest commit is better. Besides the 48px container padding, there was quite a lot of stuff in the themes, transition and style, that I removed just because of the reduced CSS complexity and the state of browsers today vs. when deck.js was first written.

@twitwi
Copy link
Contributor Author

twitwi commented Nov 12, 2013

@imakewebthings Thanks, I'll have give it a try later today, hopefully.

@twitwi
Copy link
Contributor Author

twitwi commented Nov 13, 2013

@imakewebthings nice updates. The padding on >.slide still breaks fit but it's fit's fault this time (I'll try to get the proper size of of jquery).

@twitwi
Copy link
Contributor Author

twitwi commented Nov 13, 2013

and I can't get around it for now...

@imakewebthings
Copy link
Owner

@twitwi Guess it wasn't as simple as .replace /innerWidth/g, 'width'?

@twitwi
Copy link
Contributor Author

twitwi commented Nov 13, 2013

not exactly (or I did it wrong)… it seems the css-transform plays strange with the padding. I'll have to dig more.

@twitwi
Copy link
Contributor Author

twitwi commented Nov 20, 2013

For the record, it is fixed. It was almost as easy as expected (but I needed to take a step back I guess).
I'm rolling a 1.0 bundle of my stuff and will try to advertise deck.js 1.0 a little.
twitwi/deck.js@04ff5b7

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.

None yet

2 participants