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

Issue 30871002: sao: Unify toolbar and menu definition (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 months, 1 week ago by xcodinas
Modified:
2 weeks, 6 days ago
Reviewers:
pokoli, rietveld-bot, ced, reviewbot
Visibility:
Public.

Description

- Use a unique definition - Update states of both toolbar and menu - Use a more logical order issue5644 COLLABORATOR=cedric.krier@b2ck.com

Patch Set 1 #

Patch Set 2 : Add new button, remove innecessary code, fix import /export #

Patch Set 3 : Remove export as it only needs read access #

Patch Set 4 : Fix comments #

Total comments: 2

Patch Set 5 : Use same function as buttons #

Total comments: 1

Patch Set 6 : Use different dictionary #

Total comments: 1

Patch Set 7 : Merge definitions, add new and save buttons #

Patch Set 8 : Keep original order #

Patch Set 9 : Assign link only if there is a function #

Total comments: 2

Patch Set 10 : Use a single loop #

Total comments: 2

Patch Set 11 : Use object for toolbar item, set correct state, new layout #

Unified diffs Side-by-side diffs Delta from patch set Stats (+182 lines, -132 lines) Patch
M src/tab.js View 1 2 3 4 5 6 7 8 9 10 6 chunks +150 lines, -100 lines 0 comments Download
M src/view/form.js View 1 2 3 4 5 6 7 8 9 10 2 chunks +32 lines, -32 lines 0 comments Download

Messages

Total messages: 28
xcodinas
9 months, 1 week ago (2016-12-14 08:36:31 UTC) #1
reviewbot
flake8 OK URL: https://codereview.tryton.org/30871002
9 months, 1 week ago (2016-12-14 09:02:31 UTC) #2
xcodinas
Add new button, remove innecessary code, fix import /export
9 months, 1 week ago (2016-12-14 11:04:31 UTC) #3
xcodinas
Remove export as it only needs read access
9 months, 1 week ago (2016-12-14 11:11:45 UTC) #4
reviewbot
flake8 OK URL: https://codereview.tryton.org/30871002
9 months, 1 week ago (2016-12-14 11:22:23 UTC) #5
ced
Same as tryton
9 months ago (2016-12-16 23:07:11 UTC) #6
xcodinas
Fix comments
8 months, 3 weeks ago (2016-12-29 11:43:03 UTC) #7
reviewbot
flake8 OK URL: https://codereview.tryton.org/30871002
8 months, 3 weeks ago (2016-12-29 12:00:41 UTC) #8
ced
https://tryton-rietveld.appspot.com/30871002/diff/50001/src/tab.js File src/tab.js (right): https://tryton-rietveld.appspot.com/30871002/diff/50001/src/tab.js#newcode12 src/tab.js:12: this.menu = {}; Why not at the same place ...
8 months, 2 weeks ago (2017-01-04 10:40:43 UTC) #9
xcodinas
Use same function as buttons
8 months, 2 weeks ago (2017-01-04 11:01:17 UTC) #10
reviewbot
flake8 OK URL: https://codereview.tryton.org/30871002
8 months, 2 weeks ago (2017-01-04 11:14:29 UTC) #11
ced
About the title, it is not the screen menu but the tab/form. https://tryton-rietveld.appspot.com/30871002/diff/70001/src/tab.js File src/tab.js ...
8 months, 2 weeks ago (2017-01-05 00:27:25 UTC) #12
xcodinas
Use different dictionary
8 months, 2 weeks ago (2017-01-05 12:00:11 UTC) #13
reviewbot
flake8 OK URL: https://codereview.tryton.org/30871002
8 months, 2 weeks ago (2017-01-05 12:19:02 UTC) #14
xcodinas
Merge definitions, add new and save buttons
8 months ago (2017-01-20 11:21:54 UTC) #15
reviewbot
flake8 OK URL: https://codereview.tryton.org/30871002
8 months ago (2017-01-20 11:40:13 UTC) #16
xcodinas
Keep original order
8 months ago (2017-01-20 11:49:18 UTC) #17
reviewbot
flake8 OK URL: https://codereview.tryton.org/30871002
8 months ago (2017-01-20 12:16:01 UTC) #18
pokoli
https://tryton-rietveld.appspot.com/30871002/diff/90001/src/tab.js File src/tab.js (right): https://tryton-rietveld.appspot.com/30871002/diff/90001/src/tab.js#newcode44 src/tab.js:44: this.menu_buttons[func] = link; is realy usefull to store the ...
6 months, 1 week ago (2017-03-14 10:48:59 UTC) #19
xcodinas
Assign link only if there is a function
6 months ago (2017-03-22 12:22:42 UTC) #20
reviewbot
flake8 OK URL: https://codereview.tryton.org/30871002
6 months ago (2017-03-22 12:34:50 UTC) #21
ced
https://tryton-rietveld.appspot.com/30871002/diff/140001/src/tab.js File src/tab.js (right): https://tryton-rietveld.appspot.com/30871002/diff/140001/src/tab.js#newcode98 src/tab.js:98: if (tool && !tool[4]) { Could convert all tool ...
5 months, 3 weeks ago (2017-03-30 14:30:12 UTC) #22
xcodinas
Use a single loop
2 months ago (2017-07-17 08:37:50 UTC) #23
reviewbot
flake8 OK URL: https://codereview.tryton.org/30871002
2 months ago (2017-07-17 09:10:46 UTC) #24
ced
https://tryton-rietveld.appspot.com/30871002/diff/160001/src/tab.js File src/tab.js (right): https://tryton-rietveld.appspot.com/30871002/diff/160001/src/tab.js#newcode764 src/tab.js:764: if(this.buttons[button]) { space after 'if' https://tryton-rietveld.appspot.com/30871002/diff/160001/src/tab.js#newcode777 src/tab.js:777: ['new', 'save'].forEach(function(button) ...
1 month, 3 weeks ago (2017-07-27 17:10:31 UTC) #25
ced
Use object for toolbar item, set correct state, new layout
1 month, 3 weeks ago (2017-07-27 17:16:20 UTC) #26
reviewbot
flake8 OK URL: https://codereview.tryton.org/30871002
1 month, 3 weeks ago (2017-07-27 17:38:53 UTC) #27
rietveld-bot_tryton.org
2 weeks, 6 days ago (2017-08-30 16:19:07 UTC) #28
New changeset c2b74d2ff7ed by Cédric Krier in branch 'default':
Unify toolbar and menu definition
http://hg.tryton.org/sao/rev/c2b74d2ff7ed
Sign in to reply to this message.

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