Skip to content

Conversation

@ivanyschen
Copy link
Contributor

Used bind method to enable functionalities:

  1. press enter key to login in login view
  2. press enter key to login again in login failure view.

I was thinking to add a functionality pressing enter key to send message too. But in this case, there might be conflict with occasion when users just wanna start a new line in the message. How do you guys feel about it?

@CSaratakij
Copy link
Contributor

CSaratakij commented Oct 15, 2017

@ujhuyz0110 It’s a nice feature but can you please move those bind into _initialize_view method to make its code more clean look?
and remove .swp (temponary file)

@ivanyschen
Copy link
Contributor Author

@CSaratakij You got it. I've moved them to _initialize_view and deleted the .swp.

@CSaratakij
Copy link
Contributor

@ujhuyz0110 so close... , that is in init of data not view.
I mean -> in _initialize_view.
Because if you realize, master is a part of view.
and _initialize is a init of data thats not a part of view.
see the line “self.master.title(“fbChat”)”?
I mean to move the rest of view setting to those method xD.
and it should be self.master.
(do it again and you’ll find it more clean and make sense now.)
cheers~ 😄 )
(This make the method doing one thing. it will be easier to read afterward trust me xD)

@ivanyschen
Copy link
Contributor Author

@CSaratakij Thanks for pointing that out ;)

@iamnottheway
Copy link
Contributor

iamnottheway commented Oct 15, 2017

@ujhuyz0110 That's a nice feature you added. Any thoughts on adding a splash screen before the app loads? I have a ready-made template that I built like this https://github.com/HarowitzBlack/cool-tkinter-gui-stuff.

@CSaratakij any thoughts?

EDIT:

hey @ujhuyz0110, I just saw the changes you made. The problem is that I've added some UI changes to the app for both formlogin.py and loginfailure.py merging your repo will remove my changes (It hasn't been merged yet). Can you fix that please?

@CSaratakij
Copy link
Contributor

@HarowitzBlack Do it~ 👍

@iamnottheway
Copy link
Contributor

@CSaratakij cool! I'm on it 😁

@ivanyschen
Copy link
Contributor Author

Yes! I will add this to the latest version later today

@ivanyschen
Copy link
Contributor Author

Please close #42. I have created another PR to add the functionality to the new interface.

@ivanyschen ivanyschen closed this Oct 16, 2017
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.

3 participants