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 page height issues on mobile #422

Merged
merged 8 commits into from
Jul 16, 2020
Merged

Conversation

Pireax
Copy link

@Pireax Pireax commented Jul 12, 2020

Description:

Fixes the page height issue by removing the height constrain on the html tag and allowing the height to be the page height. The background gradient's position has been adjusted to keep the old scrolling background.

Related issue (if applicable): fixes #415

Checklist:

  • The code change is tested and works locally.
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation if necessary
  • There is no commented out code in this PR.
  • My changes generate no new warnings (check the console)

@@ -98,6 +98,10 @@ table {
border-spacing: 0;
}

html {

Choose a reason for hiding this comment

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

Correct place for this since the other one isn't read at all

@datdamnzotz datdamnzotz added area/application Task related to orcpub application itself bug Something isn't working labels Jul 12, 2020
@datdamnzotz datdamnzotz added this to the 2.5.0.17 milestone Jul 12, 2020
Copy link

@codeGlaze codeGlaze left a comment

Choose a reason for hiding this comment

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

Changing body to auto fixed it on my end, I just can't find the actual source to try the change in.

I've edited it in I think... 4 files and a separate style sheet keeps getting generated somewhere I'm not accounting for.

@Pireax
Copy link
Author

Pireax commented Jul 12, 2020

@codeGlaze Yes that's similar to this, here the html's height is set to the default (which is auto), all the other changes are to fix the background image, without these additional changes the background would stretch across the whole page instead of over the visible screen area.

@codeGlaze
Copy link

I didn't notice a change in background with html set to 100% (useful work around for scaling iirc?) And body set to auto/default.

Would that about the issues you were running into?

@Pireax
Copy link
Author

Pireax commented Jul 12, 2020

Ok so I tried out setting just the body height to auto through the devtools on the live site as I think you suggested and it fixes the white region as the body now takes on the page height but the background gradient also stretches the whole page as a result of this so you would still need to fix the background.

EDIT: It's
image

vs

image

@codeGlaze
Copy link

Thanks for checking it out for me.
Nowhere near a PC right now. :)

Are you on the discord?

@datdamnzotz
Copy link

I'll tell you what, I'll toss all these branches on dev.dungeonmastersvault.com and everyone can hit it with their phones : ]

@Pireax
Copy link
Author

Pireax commented Jul 12, 2020

Which one? I just joined the Homebrew sharing one

@datdamnzotz
Copy link

Which one? I just joined the Homebrew sharing one

That would be the one. : ]

@Pireax Pireax force-pushed the page-height-fix branch from 5952d1b to 6303e53 Compare July 12, 2020 23:33
@Pireax
Copy link
Author

Pireax commented Jul 12, 2020

Alright so after much trial and error I've come up with the following: By default have no given height on the html element such that the browser itself can handle scrolling to fix the issues on mobile and use a new css class h-full to make a page fullscreen.

The issue is that the scrollable page needs no height in the html as otherwise we get issues during scrolling while the fullscreen pages didn't work without a set height at some parent level which was not possible due to the above... Normally you can use something like height: 100vh but on mobile this is the height including the address bar, making the page larger than the viewport size so the innerHeight is used instead if possible. I hope this compromise will work.

@datdamnzotz
Copy link

Let's knock the spacing down for the buttons since on mobile it still squishes them

image

Update the sytles\core.clj and add

[:.m-2
    {:margin "2px"}]

Right above this line:
https://github.com/Orcpub/orcpub/blob/develop/src/clj/orcpub/styles/core.clj#L207

and update
https://github.com/Orcpub/orcpub/blob/develop/src/cljs/orcpub/dnd/e5/views.cljs#L258

From:

[:div.f-w-b.f-s-14.t-a-c.header-tab.m-5.posn-rel

To

[:div.f-w-b.f-s-14.t-a-c.header-tab.m-2.posn-rel

Looks much better with that added
image

This doesn't affect desktop much
Before:
image

After:
image

@datdamnzotz
Copy link

Other than that.. Looks good.

@datdamnzotz
Copy link

datdamnzotz commented Jul 15, 2020

Copy link

@datdamnzotz datdamnzotz left a comment

Choose a reason for hiding this comment

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

👍

@datdamnzotz datdamnzotz merged commit 7087ed2 into Orcpub:develop Jul 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/application Task related to orcpub application itself bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Page height on mobile is too small when address bar is not visible
3 participants