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

panels for too small viewport #188

Merged
merged 1 commit into from May 21, 2016
Merged

panels for too small viewport #188

merged 1 commit into from May 21, 2016

Conversation

@dg
Copy link
Member

dg commented May 21, 2016

No description provided.

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented May 21, 2016

Super cool but unfortunately a BC breaks for every panel that assumed scrollbars on .tracy-inner. So for example Nextras Mail Panel is broken by this change.


Not only it is a BC break, I don't think I can make Nextras Mail Panel compatible without some very ugly JS hacking. By moving the overflow to element I can't style I am basically fucked.

@adrianbj

This comment has been minimized.

Copy link
Contributor

adrianbj commented May 21, 2016

@JanTvrdik - oh yeah - I didn't see that - all my custom panels use tracy-inner like that too. Now the title at the top of the panel scrolls with the content. I actually tried this approach initially when trying to deal with this and came to the same realization :)

@dg

This comment has been minimized.

Copy link
Member Author

dg commented May 21, 2016

What exactly is the problem?

@adrianbj

This comment has been minimized.

Copy link
Contributor

adrianbj commented May 21, 2016

There may be more to it in @JanTvrdik's case, but for me it is simply that you can't keep the panel <h1> header sticky at the top because the entire panel is now scrolling, not just tracy-inner. This prevent easy access to by able to move, close, and swap to a popup window without having to scroll to the top of the panel.

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented May 21, 2016

@dg I need to have (for Tracy 2.3) reserved space for vertical scrollbar to prevent the ugly horizontal scrollbar.

#tracy-debug .tracy-panel.tracy-mode-peek .tracy-inner.nextras-mail-panel,
#tracy-debug .tracy-panel.tracy-mode-float .tracy-inner.nextras-mail-panel {
    overflow-y: scroll;  /* reserve space for scrollbar */
}

This will make the panel in Tracy 2.4 visually inconsistent with others when viewport is large (because it will keep scrolling on .tracy-inner) and broken with ugly triple scrollbar when viewport is small

image

@dg

This comment has been minimized.

Copy link
Member Author

dg commented May 21, 2016

@adrianbj It is intention because in small viewport header takes big amount of space. Dragging is now possible via border.

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented May 21, 2016

It is intention because in small viewport header takes big space for itself.

The problem is that it makes the header hard to access even when the viewport is large (height 500px+) and the panel scrolls simple because there are lot of data.

@dg

This comment has been minimized.

Copy link
Member Author

dg commented May 21, 2016

Its pity. I don't know how to solve #184.

@adrianbj

This comment has been minimized.

Copy link
Contributor

adrianbj commented May 21, 2016

I am also having the horizontal scroll issue and also agree that you need access to the header all the time.

I think that #184 (comment) is a decent start to the solution - it just also needs to be called when the panel is displayed, not just when the browser is resized.

@dg

This comment has been minimized.

Copy link
Member Author

dg commented May 21, 2016

It should be solved in CSS, problem is that you cannot define max-width: 500px and max-width: 100vh at the same time.

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented May 21, 2016

@dg Just an idea – what about applying calc(100vh - 22px) based on media query on height?

@dg dg force-pushed the dg:pull-height branch from f979c4d to e24595d May 21, 2016
@dg

This comment has been minimized.

Copy link
Member Author

dg commented May 21, 2016

@JanTvrdik YES!

@dg dg force-pushed the dg:pull-height branch from e24595d to c840b6f May 21, 2016
@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented May 21, 2016

👍 Works quite well. Now I'm just not sure whether we need the first commit

@adrianbj

This comment has been minimized.

Copy link
Contributor

adrianbj commented May 21, 2016

I am finding that I need 75, rather than 25:

max-height: calc(100vh - 75px);

It is still hidden behind the debug bar, but at least if you move left of the bar, you can see it all. With 25 I still have 50x below the bottom of the viewport.

Chrome/OSX

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented May 21, 2016

@adrianbj The updated commit uses 55px. It assumes that outside of .tracy-inner is only a single <h1> element.

@adrianbj

This comment has been minimized.

Copy link
Contributor

adrianbj commented May 21, 2016

With 55px I still can't see the bottom of the panel.

screen shot 2016-05-21 at 12 37 14 pm

That's the unmodified System Info which only has a single <h1>

I just checked on Firefox and it actually looks ok.

The other problem highlighted by that screenshot is that when vertical scrollbar appears, the horizontal one also kicks in.

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented May 21, 2016

I still can't see the bottom of the panel.

That's because of the horizontal scrollbar. The only reliable solution AFAIK is to add overflow-y: scroll to .tracy-inner.

@adrianbj

This comment has been minimized.

Copy link
Contributor

adrianbj commented May 21, 2016

Actually it's not just because of the horizontal scrollbar - here it is in firefox - scrollbar is there, but you can still see the bottom of the panel - note the rounded corners that can't be seen in the Chrome screenshot.

screen shot 2016-05-21 at 12 56 52 pm

@dg

This comment has been minimized.

Copy link
Member Author

dg commented May 21, 2016

@adrianbj can you try to set overflow-y: scroll to .tracy-inner ?

@adrianbj

This comment has been minimized.

Copy link
Contributor

adrianbj commented May 21, 2016

Obviously that gets rid of the horizontal scrollbar, but it still doesn't let me see the bottom of the panel in Chrome.

@dg

This comment has been minimized.

Copy link
Member Author

dg commented May 21, 2016

55px works fine in my Chrome 51, Firefox 46 and Edge on Windows, your Firefox on Mac, but it is different on your Chrome/Mac, right? And headers are exactly the same in both pictures. Weird…

I think we should use 55px because it works fine in the most browsers.

@dg dg force-pushed the dg:pull-height branch from c840b6f to fdc980d May 21, 2016
@dg dg merged commit b2e89a2 into nette:master May 21, 2016
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@dg dg deleted the dg:pull-height branch May 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.