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

Do not display the sidebar on sign-in page #420

Merged
merged 1 commit into from
Jun 21, 2016
Merged

Conversation

astorije
Copy link
Member

@astorije astorije commented Jun 20, 2016

The sidebar is really useless on the authentication page. The settings page has no effect until logged, while icons to add a network and log out are purely wrong.
This is an attempt to make the sign-in page use the whole viewport:

The commit also:

  • Removes burger menu icon on mobile version of sign-in page
  • Adds the .signed-out class to the initial body instead of only when sign-in has failed
  • Removes hiding connect and logout button icons, which was buggy and is now useless anyway

Result

screen shot 2016-06-20 at 01 51 11

#### Current issues - [x] ~~The sign-in icon is not even displayed anymore, and has no value, so I removed _most_ of it. Because of how the client displays the sign-in form, I cannot simply remove it: when receiving the `auth` event, a click is emitted on the button that activates the right window, changes the page title, ... Not pretty. Any suggestion on improving this? I'm thinking of moving that logic to a different function that the click handler calls, and the `auth` event handler could call it without resorting to a click. Does that sound any better?~~ Scratch that, I'll tackle this in a further PR. - [x] ~~Right now, there is no scrollbar on the sign-in page, which is a problem. I'll look into that issue, but if someone sees something obvious, please speak up :-)~~ Fixed along with implementing @xPaw's suggestion. - [x] ~~The loading page looks gross with this. I should apply behavior of this PR to the loading page as well (no sidebar), and make sure the input field does not apply there either.~~ Thanks to @xPaw's suggestion, this PR also applies to the loading page \o/

@astorije astorije added Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. Meta: Do Not Merge This PR should not be merged. labels Jun 20, 2016
@xPaw
Copy link
Member

xPaw commented Jun 20, 2016

Instead of moving the whole layout, can't you just hide sidebar and footer when signed-out class is set?

@astorije
Copy link
Member Author

Good call, I'll try that.

@astorije astorije force-pushed the astorije/sign-in-fs branch from 1317700 to 198f2f3 Compare June 21, 2016 05:08
Also:
- Remove burger menu icon on mobile version of sign-in page
- Add the .signed-out class to the initial body instead of only when
  sign-in has failed
- Remove hiding connect and logout button icons, which was buggy and is
  now useless anyway
@astorije astorije force-pushed the astorije/sign-in-fs branch from 198f2f3 to 1f4e2b4 Compare June 21, 2016 05:17
@astorije astorije removed the Meta: Do Not Merge This PR should not be merged. label Jun 21, 2016
@astorije
Copy link
Member Author

Much better indeed, @xPaw, thanks for the suggestion!

@astorije astorije added this to the 2.0.0 milestone Jun 21, 2016
@xPaw
Copy link
Member

xPaw commented Jun 21, 2016

Indeed much better, will test and merge.

@xPaw
Copy link
Member

xPaw commented Jun 21, 2016

Looks fine, however I'd like to display The Lounge somewhere, otherwise it's hard to tell what you are signing into. Maybe make use of the header and put in a title there?

@astorije
Copy link
Member Author

@xPaw, I agree with the intent, but I'd rather see this in its own PR, especially since this was not available before this PR (actually, you could see this by going to the settings and reading the About statement, but that's a side effect, really).

If you want, I can change from "Sign in" to "Sign in to The Lounge" until this is taken up (up for grabs?) and removed from both the loading and the sign in pages. What do you think?

@xPaw xPaw merged commit 59c2494 into master Jun 21, 2016
@xPaw xPaw self-assigned this Jun 21, 2016
@xPaw xPaw deleted the astorije/sign-in-fs branch June 21, 2016 17:10
astorije added a commit that referenced this pull request Sep 18, 2016
This change was originally made in #420 then I broke it for themes in #615, sigh...
astorije added a commit to nornagon/lounge that referenced this pull request Oct 23, 2016
This change was originally made in thelounge#420 then I broke it for themes in thelounge#615, sigh...
matburnham pushed a commit to matburnham/lounge that referenced this pull request Sep 6, 2017
Do not display the sidebar on sign-in page
matburnham pushed a commit to matburnham/lounge that referenced this pull request Sep 6, 2017
This change was originally made in thelounge#420 then I broke it for themes in thelounge#615, sigh...
@xPaw xPaw removed their assignment Mar 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants