Am 05/08/2018 um 09:22 AM schrieb Dominik Csapak:
looks good in general, a few comments inline (most are nitpicks)
On 05/07/2018 05:41 PM, René Jochum wrote:
As given in the subject this implements role create/update/delete over
the manager.
There's currently no coler highlightning for "special" roles.
I decided to call "special" roles "Builtin" any feedback on that is much
appreciated.
I like Builtin, is more expressive about what they really are, as they
ain't that special they're just, as you say, builtin, so good
suggestion! Maybe use 'Built-In', seems more used doing a quick internet
search, but I'm actually OK with both.
[snip]
@@ -58,8 +90,34 @@ Ext.define('PVE.dc.RoleView', {
listeners: {
activate: function() {
store.load();
+ },
+ itemdblclick: run_editor,
+ },
+ tbar: [
+ {
+ text: gettext('Create'),
+ handler: function() {
+ var win = Ext.create('PVE.dc.GroupEdit', {});
this should be 'PVE.dc.RoleEdit'
Was wondering why nothing popped up after creation 'til I saw
Create: Group in the window title ^^
+ win.on('destroy', reload);
+ win.show();
+ }
+ },
+ {
+ xtype: 'proxmoxButton',
+ text: gettext('Edit'),
+ disabled: true,
+ selModel: sm,
+ handler: run_editor
for this we probably want an 'enableFn' so that we only enable the edit
button on non builtin entries, like this:
enableFn: function(record) {
return record.data.special !== '1';
}
Same for the Remove button below, please.
We cannot remove builtin roles, so there it should be disabled.
Oh, and personally it would make the creation window wider, I get
scrollbars here and it looks a bit stuffed once multiple permissions are
selected, I know that it won't fix it entirely as they will be always
cut off, but now it looks just too stuffed, IMO.
+ },
+ {
+ xtype: 'proxmoxStdRemoveButton',
+ selModel: sm,
+ callback: function() {
+ reload();
+ },
+ baseurl: '/access/roles/'
}
- }
+ ]
});
me.callParent();
_______________________________________________
pve-devel mailing list
[email protected]
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
