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

login form added #43

Merged
merged 3 commits into from
Oct 13, 2016
Merged

login form added #43

merged 3 commits into from
Oct 13, 2016

Conversation

agrim123
Copy link
Member

Added a login screen in accordance to issue #10 . a live preview is available on https://slack12345.herokuapp.com (this is my instance of jinora for testing purpose)

@tnmy44
Copy link
Member

tnmy44 commented Aug 23, 2016

Nice work! Can you make few changes, though:

  • Set focus on the text box by default when the page is opened.
  • User should be able to press the enter key in place of clicking on the "Start Chatting" button.
  • If the user enters an empty string, assign a random name from the defaultNames array.

@kdexd
Copy link

kdexd commented Aug 23, 2016

I haven't seen the diffs till now, but it really looks good already ! 😄

@kdexd
Copy link

kdexd commented Oct 13, 2016

Can this be merged ?

@ketanhwr
Copy link
Member

Damn this looks nice. 👍

@tnmy44 tnmy44 merged commit a85e9c8 into sdslabs:master Oct 13, 2016
@@ -1,34 +1,41 @@
@import url(http://fonts.googleapis.com/css?family=Source+Sans+Pro:200,300);
Copy link
Contributor

Choose a reason for hiding this comment

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

It gives the following warning The page at 'https://chat.sdslabs.co/' was loaded over HTTPS, but requested an insecure stylesheet 'http://fonts.googleapis.com/css?family=Source+Sans+Pro:200,300'. This content should also be served over HTTPS.

Copy link
Member Author

Choose a reason for hiding this comment

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

changing it to https would do

Copy link
Member

Choose a reason for hiding this comment

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

Fixed with a7ddfba.

Copy link
Member

Choose a reason for hiding this comment

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

A better fix would be to use //fonts instead of http://fonts or https://fonts.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@DhavalKapil DhavalKapil Oct 14, 2016

Choose a reason for hiding this comment

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

My bad, This article makes a lot of sense.

@csoni111
Copy link
Contributor

csoni111 commented Oct 13, 2016

Another issue is that, if you ban a nick from slack. It still gets accepted on the login screen, but only when the user tries to send a message with the banned nick. He gets that old prompt box asking for another username.

I have mentioned it here #44

@gupta-utkarsh
Copy link
Member

gupta-utkarsh commented Oct 13, 2016

@csoni111 It would be nice if you could file a separate issue for the above bug. :)

@rishidvgn
Copy link

Hi people,
I am a former member of sdslabs, but due to some reason this thread still
has me in the mailing list. Please append the group and list.

Warm regards,
Rishi Devgan

On 14-Oct-2016 1:05 AM, "Utkarsh Gupta" [email protected] wrote:

@csoni111 https://github.com/csoni111 It would be nice if you could
file a separate issue for this. :)


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#43 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGGI98epTbvNdx69d91bV6d0QsqXjC9dks5qzofsgaJpZM4Jq6J3
.

@rishidvgn
Copy link

Never mind, I just unsubscribed myself.

On 14-Oct-2016 1:07 AM, "Rishi Devgan" [email protected] wrote:

Hi people,
I am a former member of sdslabs, but due to some reason this thread still
has me in the mailing list. Please append the group and list.

Warm regards,
Rishi Devgan

On 14-Oct-2016 1:05 AM, "Utkarsh Gupta" [email protected] wrote:

@csoni111 https://github.com/csoni111 It would be nice if you could
file a separate issue for this. :)


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#43 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGGI98epTbvNdx69d91bV6d0QsqXjC9dks5qzofsgaJpZM4Jq6J3
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants