davidskalinder / mpeds-coder

MPEDS Annotation Interface
MIT License
0 stars 0 forks source link

Get rid of fixed width for sticky class #94

Closed davidskalinder closed 4 years ago

davidskalinder commented 4 years ago

Since I'm re-enabling the sticky JS, I'd like to take this out:

https://github.com/davidskalinder/mpeds-coder/blob/c9292d6501211fec855d00107b24be166ed9b3bc/static/style.css#L7

@alexhanna, can you remember why that's in there in the first place? The coding controls are often too wide to fit in 250px, and otherwise it seems to just create extra whitespace around the control panels and make it so they don't resize into small windows as well? But I am in general a CSS relative-unit supremacist, so maybe that's blinding me to the fact that there's some good reason for this?

I'm going to put this in a separate branch so that we can keep it out of any PRs, but I'm having trouble seeing how it's better to get rid of that line altogether?...

alexhanna commented 4 years ago

I can't say I remember why. Feel free to nix it.

davidskalinder commented 4 years ago

Lol, I just realized that the answer I was really hoping for was "I vividly remember the exact reason, and I guarantee that that reason is no longer valid." But I'll take it! Like I say I'll PR it separately so you can nix the nixing if anything goes wrong.

alexhanna commented 4 years ago

Any CSS/JS stickiness was probably motivated by Stack Overflow copy-pasta.

davidskalinder commented 4 years ago

Well, this one doesn't have much of anything to do with the stickiness -- it just sets the width of the coding and event description panels (which happen to be in divs with class "sticky"). But anyway yeah see what you think when you see it.

davidskalinder commented 4 years ago

Fixed in branch ending at 001b2cf9. Merged into testing and master (with some easily-resolved conflicts). Deployed into testing UI.

davidskalinder commented 4 years ago

Fixed in branch ending at 001b2cf. Merged into testing and master (with some easily-resolved conflicts). Deployed into testing UI.

Whoops, wrong issue. This one is fixed in 881f7fd6 and merged as above.

davidskalinder commented 4 years ago

We'll test this along with #51.

davidskalinder commented 4 years ago

Tested and deployed.