Re: [pmg-devel] [PATCH pmg-gui 3/4] quarantine: add common controller for all quarantines

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 20/10/2022 um 21:14 schrieb Stoiko Ivanov:
> over the time the spam quarantine has gained quite a few nice
> usability improvments and features, which would be useful in
> the virus and attachment quarantines as well - e.g.:
> 
> * multi-mail selection
> * keyboard actions
> * context menu
> * download mail as .eml
> 
> by adding the common class (which is mostly a copy of the controller
> part of SpamQuarantine.js with changes of 'var' to 'let' and removal of the
> spam-specific parts), we can reuse it for all the quarantines.
> 

great re-use! Can you please move the original implementation in SpamQuarantine
directly over in this path, that'd would better separate the "factor out" and
"add new functionality to components" parts.

> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  js/Makefile                           |   1 +
>  js/controller/QuarantineController.js | 170 ++++++++++++++++++++++++++
>  2 files changed, 171 insertions(+)
>  create mode 100644 js/controller/QuarantineController.js
> 
> diff --git a/js/Makefile b/js/Makefile
> index b904598..9a2bcf2 100644
> --- a/js/Makefile
> +++ b/js/Makefile
> @@ -19,6 +19,7 @@ JSSRC=							\
>  	RuleInfo.js					\
>  	RuleEditor.js					\
>  	MainView.js					\
> +	controller/QuarantineController.js		\
>  	QuarantineContextMenu.js			\
>  	QuarantineList.js				\
>  	SpamInfoGrid.js					\
> diff --git a/js/controller/QuarantineController.js b/js/controller/QuarantineController.js
> new file mode 100644
> index 0000000..dfe2915
> --- /dev/null
> +++ b/js/controller/QuarantineController.js
> @@ -0,0 +1,170 @@
> +Ext.define('PMG.controller.QuarantineController', {

I'd drop the 'Controller' at the end, if one uses the full path to instantiate/refer to this,
it's already clear that it's a controller due to the middle part.

> +    extend: 'Ext.app.ViewController',
> +    xtype: 'controller.Quarantine',

xtype doesn't gets the controller. part, and please keep our general style of prefixing with
the product abbrev, e.g.: 'pmgQuarantine'

> +    alias: 'controller.quarantine',

I guess both as the "wrong" xtype did not work? anyhow, either xtype or alias, but
not both. I've lessend my strong preference of one of them more recently, so I don't
care much which one, more for only using one, not both.