On 2/2/23 11:56, Thomas Lamprecht wrote:
looks like going in the right direction and feasible, some comments inline
high level: subject is using module name 1:1, as mentioned a few times please
avoid that, for ~ 99.9% cases it's not useful, for sure not for d/changelog but
not really for other devs in `git log` (sorting trees can mean widely different
things), maybe:
ui: add edit window for changing resource tree sort behavior
yes, sorry, for adding new widgets i still default in my head to just
adding the class name...
And for the module/file name I maybe wouldn't imply that this is for sorting
only, I could immagine that we expose other knobs for the tree behavior or
visuals in the future (e.g., add more font-weight to names, or
yeah sure, does 'TreeSettingsEdit' (or 'TreeOptionsEdit') make sense?
Am 01/02/2023 um 16:49 schrieb Dominik Csapak:
in cluster options (for datacenter.cfg) and besides the view selector
(for the browser local storage)
I'd rather do it browser-local storage only, that way we can play a bit
around possbly adding or changing knob (names) and once stable for a time
and users are happy we can commit to adding this to datacenter config w
yeah sure
the edit window is the same for bot but either makes an api call or
saves the resulting values into the browser local storage.
Signed-off-by: Dominik Csapak <d.csa...@proxmox.com>
---
not sure about just showing 'Default' for the options in both cases.
I could make the text different for the datacenter options (there it's
fixed) and make the text dynamic for the browser local storage, but it
does not seem worth it really...
could also always show the inherent defaults, but that would be wrong
for the local storage when something is set in the datacenter config...
any commend on this?
www/manager6/Makefile | 1 +
www/manager6/Utils.js | 2 +-
www/manager6/Workspace.js | 27 +++++-
www/manager6/dc/OptionView.js | 10 +++
www/manager6/window/TreeSortingEdit.js | 113 +++++++++++++++++++++++++
5 files changed, 150 insertions(+), 3 deletions(-)
create mode 100644 www/manager6/window/TreeSortingEdit.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 05afeda40..157d4da52 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -119,6 +119,7 @@ JSSRC=
\
window/ScheduleSimulator.js \
window/Wizard.js \
window/GuestDiskReassign.js \
+ window/TreeSortingEdit.js \
ha/Fencing.js \
ha/GroupEdit.js \
ha/GroupSelector.js \
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index ee9e4bd5d..0c57e0844 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1865,7 +1865,7 @@ Ext.define('PVE.Utils', {
PVE.UIOptions = {
'allowed-tags': [],
};
- for (const option of ['allowed-tags', 'console', 'tag-style']) {
+ for (const option of ['allowed-tags', 'console', 'tag-style',
'tree-sorting']) {
PVE.UIOptions[option] = response?.result?.data?.[option];
a thought: why doesn't "PVE.UIOptions" real, i.e., a full fledged module that
hosts the respective functions and state that now is (mostly?) in Utils?
there wasn't that much in there, but i agree it's own module does make more
sense,
i'll do a refactor in a v2
}
diff --git a/www/manager6/Workspace.js b/www/manager6/Workspace.js
index d0f7ff760..1963dc3f2 100644
--- a/www/manager6/Workspace.js
+++ b/www/manager6/Workspace.js
@@ -223,7 +223,9 @@ Ext.define('PVE.StdWorkspace', {
let appState = Ext.create('PVE.StateProvider');
Ext.state.Manager.setProvider(appState);
- let selview = Ext.create('PVE.form.ViewSelector');
+ let selview = Ext.create('PVE.form.ViewSelector', {
+ flex: 1,
+ });
let rtree = Ext.createWidget('pveResourceTree', {
viewFilter: selview.getViewFilter(),
@@ -449,7 +451,28 @@ Ext.define('PVE.StdWorkspace', {
margin: '0 0 0 5',
split: true,
width: 300,
- items: [selview, rtree],
+ items: [
+ {
+ xtype: 'container',
+ layout: 'hbox',
+ padding: '0 0 5 0',
+ items: [
+ selview,
+ {
+ xtype: 'button',
+ iconCls: 'fa fa-fw fa-gear',
+ handler: () => {
+
Ext.create('PVE.window.TreeSortingEdit', {
+ autoShow: true,
+ browserSettings: true,
+ apiCallDone: () =>
PVE.Utils.fireUIUpdate(),
+ });
+ },
+ },
+ ],
+ },
+ rtree,
+ ],
listeners: {
resize: function(panel, width, height) {
var viewWidth = me.getSize().width;
diff --git a/www/manager6/dc/OptionView.js b/www/manager6/dc/OptionView.js
index 2e4f76263..0f5eaa680 100644
--- a/www/manager6/dc/OptionView.js
+++ b/www/manager6/dc/OptionView.js
@@ -529,6 +529,15 @@ Ext.define('PVE.dc.OptionView', {
},
};
+ me.rows['tree-sorting'] = {
+ required: true,
+ renderer: (value) => value ? PVE.Parser.printPropertyString(value)
: gettext('No sorting overrides'),
+ header: gettext('Tree Sorting'),
+ editor: {
+ xtype: 'pveTreeSortingEdit',
+ },
+ };
+
me.selModel = Ext.create('Ext.selection.RowModel', {});
Ext.apply(me, {
@@ -565,6 +574,7 @@ Ext.define('PVE.dc.OptionView', {
}
PVE.UIOptions['tag-style'] = store.getById('tag-style')?.data?.value;
+ PVE.UIOptions['tree-sorting'] =
store.getById('tree-sorting')?.data?.value;
PVE.Utils.updateTagSettings(PVE.UIOptions['tag-style']);
PVE.Utils.fireUIUpdate();
});
diff --git a/www/manager6/window/TreeSortingEdit.js
b/www/manager6/window/TreeSortingEdit.js
new file mode 100644
index 000000000..eff7f3f21
--- /dev/null
+++ b/www/manager6/window/TreeSortingEdit.js
@@ -0,0 +1,113 @@
+Ext.define('PVE.window.TreeSortingEdit', {
+ extend: 'Proxmox.window.Edit',
+ alias: 'widget.pveTreeSortingEdit',
+ mixins: ['Proxmox.Mixin.CBind'],
+
+ title: gettext('Tree Sorting'),
+
+ isCreate: false,
+
+ url: '/cluster/options',
+
+ browserSettings: false,
is above a preparation of exposing this additonally in the Datacenter Options
panel for configuring the backend config values, not the browser local ones?
I'd probably favor doing that via two widgets, one deriving from the other - but
if we only allow brwoser local storage as starter we'd need the differentation
at all for now.
yeah i'm using that in the next patch to also expose it in the datacenter
options
but i'll drop it for browser local storage only
+
+ items: [
+ {
+ xtype: 'inputpanel',
+ cbind: {},
+ onSetValues: (values) => values?.['tree-sorting'] ?? {},
+ onGetValues: function(values) {
+ if (this.up('window').browserSettings) {
+ let sp = Ext.state.Manager.getProvider();
nit: would like to avoid very short abbreviations which could get confusing if
this ever expands or gets used as template and copied.
s/sp/stateProvider/ or maybe even s/sp/localStorage/
+ let state = values ? values : null;
+ sp.set('pve-tree-sorting', state);
could drop intermediate variable
sp.set('pve-tree-sorting', values || null);
+ return {};
+ }
+ PVE.Utils.delete_if_default(values, 'group-templates', "1");
+ PVE.Utils.delete_if_default(values, 'group-guest-types', "1");
+ // no delete in property strings
+ delete values.delete;
+
+ let value = PVE.Parser.printPropertyString(values);
+
+ if (value === '') {
+ return { 'delete': 'tree-sorting' };
+ }
+ return { 'tree-sorting': value };
+ },
+ items: [
+ {
+ name: 'sort-field',
+ xtype: 'proxmoxKVComboBox',
please put xtype first
+ fieldLabel: gettext('Sort Field'),
+ labelWidth: 120,
might want to but that in a defaults: {} config at inputpanel level
+ comboItems: [
+ ['__default__', Proxmox.Utils.defaultText],
+ ['vmid', gettext('VMID')],
do we really translate VMID, as in, is there a (updated in the last 12 months)
translation that sets this to something different and sensible?
only arabic seems to translate VMID, and not even consistent
(it's VMID in some other arabic strings) so i think we would be fine to
drop that. we're not consitently using gettext for it anyway
+ ['name', gettext('Name')],
+ ],
+ defaultValue: '__default__',
+ value: '__default__',
+ deleteEmpty: false,
+ },
+ {
+ name: 'group-templates',
+ xtype: 'booleanfield',
please put xtype first
besides that and unrelated to your series, this widget is a bit odd and as it
has no pve/proxmox prefix (I initially thought it's a ExtJS one that I just
never stumbled uppon) and should get renamed to something more telling like
pmxBooleanComboBox, probably should move to widget toolkit too
sure
+ fieldLabel: gettext('Group Templates'),
+ defaultValue: '__default__',
+ value: '__default__',
+ deleteEmpty: false,
+ labelWidth: 120,
+ },
+ {
+ name: 'group-guest-types',
+ xtype: 'booleanfield',
please put xtype first
+ fieldLabel: gettext('Group Types'),
+ defaultValue: '__default__',
+ value: '__default__',
+ deleteEmpty: false,
+ labelWidth: 120,
+ },
+ {
+ xtype: 'displayfield',
+ userCls: 'pmx-hint',
+ value: gettext('Settings are saved in the browser local
storage'),
+ hidden: true,
+ cbind: {
+ hidden: '{!browserSettings}',
+ },
+ }
+ ],
+ },
+ ],
+
+ submit: function() {
+ let me = this;
+
+ if (me.browserSettings) {
+ me.down('inputpanel').getValues();
+ me.apiCallDone();
+ me.close();
+ } else {
+ me.callParent();
+ }
+ },
+
+ initComponent: function() {
+ let me = this;
+
+ me.callParent();
+
+ me.load({
+ success: function(response) {
+ let values = response?.result?.data?.['tree-sorting'] ?? {};
+ if (me.browserSettings) {
+ let sp = Ext.state.Manager.getProvider();
+ values = sp.get('pve-tree-sorting');
+ }
+ me.down('inputpanel').setValues({ 'tree-sorting': values });
+ },
+ });
+ },
+
+});
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
