Skip to content
This repository has been archived by the owner on Oct 6, 2022. It is now read-only.

Use flexbox #29

Closed
wants to merge 19 commits into from
Closed

Use flexbox #29

wants to merge 19 commits into from

Conversation

101arrowz
Copy link
Contributor

@101arrowz 101arrowz commented Oct 27, 2019

I'm only making this PR so I don't have to later. I'll finish my changes, then you can merge.

As I mentioned in #28, Flexbox is a really handy layout format for CSS. So handy, in fact, that it allows us to finally remove the size checks and have awesome mobile support!

It's not fully fixed yet (some bugs in level viewing, not implemented in leaderboards, etc.) but it will be glorious when complete.

You can see the changes in Map Packs, Home, and Gauntlets as of now. Try resizing the screen to mobile size!

E: This is taking longer than expected, I might be done by tomorrow but not for certain.

E2: Added support for any screen size in level.html

@101arrowz
Copy link
Contributor Author

@GDColon Please respond to the PR, possibly check out the current progress. The entire point is to make mobile support possible, so maybe resize your window to small dimensions. It is nearing completion but I won't be done till tomorrow because redoing everything is tricky.

@GDColon
Copy link
Owner

GDColon commented Oct 30, 2019

Yeah, don't worry I saw it. Just waiting for you to finish.

@101arrowz
Copy link
Contributor Author

101arrowz commented Oct 30, 2019

I'm gonna stop for today, I have an academic life too. I'll most likely finish tomorrow but not for sure. This is definitely the biggest change I've made yet. Just profile to go!

@101arrowz
Copy link
Contributor Author

101arrowz commented Oct 31, 2019

Wooow that took a long time. But it's done!

This PR does a lot more than just implement flexbox. For example, it uses vmin and vmax to work better with weird aspect ratios.

Obviously it isn't perfect, but after merging we can fix any bugs that arise (I tested every screen on most aspect ratios and screen resolutions though, so we should be fine for the most part). I kinda got lazy on the profile, so it works but it's not super consistent with the other screens as much. Now mobile users can rejoice!

There's a lot of dead CSS rules that should be removed, but again, I can't be assed. We'll deal with that later.

@101arrowz 101arrowz changed the title [DON'T MERGE YET] Use flexbox Use flexbox Oct 31, 2019
@GDColon
Copy link
Owner

GDColon commented Oct 31, 2019

Amazing job! I'll look at the PR as soon as I can. Your help is greatly appreciated 😄

@GDColon
Copy link
Owner

GDColon commented Oct 31, 2019

Update: You might want to go back to the drawing board. I don't know what you were testing this on, but on my computer it looks... not great.

@101arrowz
Copy link
Contributor Author

101arrowz commented Oct 31, 2019

I probably should have elaborated: I made some design changes, mostly for compatibility with small screens. If you don't like it I'll try to emulate the original as well as possible. But that's exactly how it looks on my system, and I don't think it looks that terrible...

Tell me which sections you want changed and I'll try to do them.

E: I understand you want consistency with the real GD, but doing that involves some other sacrifices. Of course if you really want it I'll do it within a day or two.

@GDColon
Copy link
Owner

GDColon commented Oct 31, 2019

I'm.... not very happy with how it looks right now, unfortunately. You can try to improve things if you really want, but keep in mind that I want the page to be as accurate as possible to the one in GD.

@101arrowz
Copy link
Contributor Author

Try again with the most recent commit?

@GDColon
Copy link
Owner

GDColon commented Oct 31, 2019

Not much has changed.
Knowing how hard you worked, it really pains me to say it, but I don't think a merge is going to happen unless it looks exactly like it did before...

@101arrowz
Copy link
Contributor Author

101arrowz commented Oct 31, 2019

What do you want changed though? I fixed the profile page, filters page and level page. They look exactly like GD (on my phone at least).

@GDColon
Copy link
Owner

GDColon commented Oct 31, 2019

I just want it to look the same as it did before. If you get mobile working, that's a huge plus. But at the cost of making the site visually appealing?... i'm not so sure.

@101arrowz
Copy link
Contributor Author

101arrowz commented Oct 31, 2019

I'm pretty sure the level and leaderboard pags look virtually identical to before. But I really can't push my opinion since this is your project after all 😄. I'll see what I can do to get it looking the same, but mobile support is, in my humble opinion, more important than the 20-30 pixel offset in elements in the old version.

@101arrowz
Copy link
Contributor Author

101arrowz commented Oct 31, 2019

@GDColon I've made some more changes and I think it looks as appealing as the original.

I physically cannot make it the exact same as the original because I have to use changing sizes for images, text, etc. for mobile support whereas you don't in the original version. But I did the best I could and I'm sorry that it can't be perfect, but it's actually very very close to GD, some sections even closer than the original. PLEASE consider this when looking at the changes. Don't compare it to the original, compare it to the game. Even if you compare it to the original, it's within 20 pixels! If you want any more changes let me know.

Sorry if I sound desperate, it's because I kind of am. I spent way too long on this PR and I really want you to merge lol. But again it's your choice.

@101arrowz
Copy link
Contributor Author

101arrowz commented Oct 31, 2019

I'm done working for today. I've made the original and the flexbox versions as close as possible.

I'll wait for your decision, but you'd get a lot more users if you added mobile support. Remember that some of your users reported that many phones like the iPhone XS have bugs where even after rotating they can't see the content. More than that, even the original GD doesn't support portrait orientation on mobile! You create an incentive to use GDBrowser over GD itself, which is a huge plus.

In the end, however, I think I'm biased since I'm attached to my own work, and you should do what you think is best for the project, whether that means merging or not 😄

Side note: you talked on twitter about having GDBrowser allow user accounts to do stuff like post comments, but storing passwords on client side is, like, a really bad idea. If you think it's a necessary feature then use the sessionStorage API, which automatically clears at the end of a browser session. You don't want the password stored in cookies. They don't automatically clear when the session ends, only after a certain amount of time. A lot of people use the same password for their GD account as their email, and in a few browsers other websites can read cookie data from your site! This isn't the case for the (easier to use) sessionStorage. I can also make a PR for this :)

@GDColon
Copy link
Owner

GDColon commented Oct 31, 2019

I don't want you to ditch your work, so the best direction to head might be a mobile version of the site. But that might be difficult since that means two versions of the same page that have to continue to be updated. And making new pages is even worse.
Also about storing passwords, thanks for bringing that to my attention. I want users to remain logged in even after the session, but I don't want their passwords stored in a database.

@101arrowz
Copy link
Contributor Author

101arrowz commented Oct 31, 2019

@GDColon Try to pinpoint any issues with the latest commits. We could just use a single version of the site if you tell me what to fix. Have you given it another shot with the most recent leaderboard fixes, etc?

As for your concern about storing passwords, allow me to assure you that it isn't worth it to try to persist password between sessions. SSO is different from direct authentication. Trust me on this one, it's a huge liability for your users. If you absolutely need to persist between sessions, you should have the client send the password to the server, which will encrypt it using a secret key and return an encrypted version of the passcode to the client (server won't store the passcode anywhere). The client can store that encrypted passcode (which can only be used on GDBrowser and won't expose the user's actual password) and send it to the server whenever it makes a request. The server decrypts it and uses it to do whatever password-protected action is desired. I know this is complicated, so I can also submit a PR for this.

@GDColon GDColon closed this Nov 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants