Tryton Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(287)

Issue 37551002: sao: Add login details in title

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 weeks ago by kstenger
Modified:
1 week ago
Reviewers:
pokoli, ced, reviewbot
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 8

Patch Set 2 : Add Sao.set_title() #

Patch Set 3 : Add CHANGELOG #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -4 lines) Patch
M CHANGELOG View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M src/sao.js View 1 3 chunks +24 lines, -4 lines 3 comments Download
M src/session.js View 1 1 chunk +1 line, -0 lines 4 comments Download

Messages

Total messages: 15
kstenger
2 weeks ago (2017-07-11 21:42:24 UTC) #1
reviewbot
flake8 OK URL: https://codereview.tryton.org/37551002
2 weeks ago (2017-07-11 22:11:41 UTC) #2
pokoli
https://tryton-rietveld-hrd.appspot.com/37551002/diff/1/src/session.js File src/session.js (right): https://tryton-rietveld-hrd.appspot.com/37551002/diff/1/src/session.js#newcode35 src/session.js:35: title += ' - ' + Sao.Session.current_session.login; this should ...
1 week, 6 days ago (2017-07-12 07:59:21 UTC) #3
kstenger
https://tryton-rietveld-hrd.appspot.com/37551002/diff/1/src/session.js File src/session.js (right): https://tryton-rietveld-hrd.appspot.com/37551002/diff/1/src/session.js#newcode35 src/session.js:35: title += ' - ' + Sao.Session.current_session.login; On 2017/07/12 ...
1 week, 6 days ago (2017-07-12 15:42:34 UTC) #4
ced
https://tryton-rietveld.appspot.com/37551002/diff/1/src/session.js File src/session.js (right): https://tryton-rietveld.appspot.com/37551002/diff/1/src/session.js#newcode35 src/session.js:35: title += ' - ' + Sao.Session.current_session.login; Maybe we ...
1 week, 6 days ago (2017-07-12 22:11:03 UTC) #5
kstenger
Add Sao.set_title()
1 week, 5 days ago (2017-07-13 15:00:19 UTC) #6
kstenger
https://tryton-rietveld.appspot.com/37551002/diff/1/src/session.js File src/session.js (right): https://tryton-rietveld.appspot.com/37551002/diff/1/src/session.js#newcode35 src/session.js:35: title += ' - ' + Sao.Session.current_session.login; On 2017/07/12 ...
1 week, 5 days ago (2017-07-13 15:00:22 UTC) #7
reviewbot
flake8 OK URL: https://codereview.tryton.org/37551002
1 week, 5 days ago (2017-07-13 15:07:20 UTC) #8
pokoli
Missing changelog
1 week, 1 day ago (2017-07-17 08:23:43 UTC) #9
kstenger
Add CHANGELOG
1 week, 1 day ago (2017-07-17 15:19:07 UTC) #10
reviewbot
flake8 OK URL: https://codereview.tryton.org/37551002
1 week, 1 day ago (2017-07-17 15:30:16 UTC) #11
ced
https://tryton-rietveld.appspot.com/37551002/diff/40001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/37551002/diff/40001/src/sao.js#newcode240 src/sao.js:240: Sao.set_title(preferences.status_bar); I think it should always be called. https://tryton-rietveld.appspot.com/37551002/diff/40001/src/sao.js#newcode296 ...
1 week, 1 day ago (2017-07-17 22:39:33 UTC) #12
kstenger
https://tryton-rietveld.appspot.com/37551002/diff/40001/src/session.js File src/session.js (right): https://tryton-rietveld.appspot.com/37551002/diff/40001/src/session.js#newcode34 src/session.js:34: Sao.set_title(''); On 2017/07/17 22:39:33, ced wrote: > I do ...
1 week, 1 day ago (2017-07-17 23:20:47 UTC) #13
pokoli
https://tryton-rietveld.appspot.com/37551002/diff/40001/src/session.js File src/session.js (right): https://tryton-rietveld.appspot.com/37551002/diff/40001/src/session.js#newcode34 src/session.js:34: Sao.set_title(''); On 2017/07/17 23:20:47, kstenger wrote: > On 2017/07/17 ...
1 week ago (2017-07-18 07:28:52 UTC) #14
ced
1 week ago (2017-07-18 07:36:16 UTC) #15
https://tryton-rietveld.appspot.com/37551002/diff/40001/src/sao.js
File src/sao.js (right):

https://tryton-rietveld.appspot.com/37551002/diff/40001/src/sao.js#newcode278
src/sao.js:278: Sao.set_title('');
May need a comment to explain why it is called.

https://tryton-rietveld.appspot.com/37551002/diff/40001/src/session.js
File src/session.js (right):

https://tryton-rietveld.appspot.com/37551002/diff/40001/src/session.js#newcode34
src/session.js:34: Sao.set_title('');
On 2017/07/18 07:28:51, pokoli wrote:
> On 2017/07/17 23:20:47, kstenger wrote:
> > On 2017/07/17 22:39:33, ced wrote:
> > > I do not see why it is needed.
> > > Also I would prefer to not have session depending on UI.
> > 
> > I put this here is because I believe it's really important to give the user
> > feedback on which username is beeing used to log-in while beeing asked for
the
> > password.
> > 
> > However I understand your concern.
> > 
> > Maybe you or Sergi can help me figure this out better, if there is a better
> > place to do it, else we can just remove it.
> 
> I don't think it's needed to update the title while logging in. The user
should
> be smart enought to remember the username entered. In case it doesnt it can
> cancel and re-enter the username. 

Agree, you can just remove this call.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cd18842