-
Notifications
You must be signed in to change notification settings - Fork 1
Description
Icinga DB Web provides its own control to let users choose between a list view's layout since its initial release. Since then, there were multiple attempts to generalize this, but only the last is, how I see it, final. See #252 and related subsequent changes.
The widget to control this however, is still part of Icinga DB Web. We should transfer this here as well now.
Considerations
Compatibility
There's not much here, only the way how it's prepared. Icinga DB Web provides \Icinga\Module\Icingadb\Web\Controller::createViewModeSwitcher() for this. Adding the same method to CompatController, like for the other controls (e.g. createSortControl()), is not possible as this would break older versions of Icinga DB Web due to the different method signature. I'd rather suggest to introduce a new trait ipl\Web\Common\Controls that provides a way to prepare the new view mode switcher. It may even be what provides the other preparation methods in the future, as requiring to extend CompatController was already not always an option, hence the generalized name Controls.
Extendability
We can find two cases where the default view modes were not sufficient. It can safely be assumed that this might be the case in the future again, in a different product. Icinga DB Web currently supports a fourth view mode: tabular. This is specific to Icinga DB Web and cannot supported by the new mode switcher. Then there is the GridViewModeSwitcher which overrides the available modes entirely to introduce its own. Both are valid use cases, though. So the new implementation should allow to add new modes as well as to remove default modes.
Integration
That's were things get difficult. These areas must be considered and still be supported.
Accessibility
The control is implemented as a group of radio elements which rely on unique IDs. With Icinga Web, these must be based on the container id as usual. This must still be possible.
Interaction With Other Controls
Icinga DB Web's view mode switcher cannot be prepared without a limit control. The reason is that changing to the minimal layout, the default limit should be 50 instead of 25. This might still be supported, but shouldn't be a requirement. The current implementation also supports Icinga DB Web's custom grid mode, so there's obviously the need for custom rules. I don't have a specific implementation in mind right now, but since the LimitControl is a part of ipl web already, it is certainly not a bad idea to try to preserve this. Not sure though, if we can or should support the case for a vertical pagination, maybe that's something only a custom rule is able to support.
Persistence
Changing the view mode in Icinga DB Web is persisted into the session/preferences of the user, dependent on where the change happened. That is, if you switch to the minimal mode in the host list, the service list still shows the default view mode, every host list though, whether the host overview, the host problems or a hosts dashlet (without an explicit view mode), all are shown in the minimal mode by default now. This is definitely not something the library can provide by default, but shouldn't make it hard to implement either. Custom events may be the answer to this. Though, at the moment this relies on form success handling, which in turn relies on when the http request is being handled. I'd like to avoid a solution like with \ipl\Web\Compat\SearchControls::callHandleRequest() at all cost!
So please let's think about an alternative. Working with promises in the past (mainly with JS), I'd imagine something like a promise that may be already settled and the handler is triggered immediately once registered. This is different to our current event implementation, as once the event has already been triggered, no handler registered later will be called anymore. This may even prove beneficial for other cases? Though, this is just an idea but I don't know if it's good or bad. Nor how it should be implemented and where. Whoever takes this on, should discuss it with @lippserd first.
But it may also be just a question of orchestration. I've already mentioned that the trait ipl\Web\Common\Controls may be home to the other preparation methods in the future. And the problem if and when the request is being handled isn't unique to the search bar nor to the new view mode switcher. So the answer to this may also be just a method ipl\Web\Common\Controls::handleControls(ServerRequestInterface): static which tracks prepared controls on its own and allows the controller to decide when the request is handled. The neat effect of course would be that custom event handlers may easily be registered beforehand.
We should find a way that's compatible with all controls right now, not just the view mode switcher. The above are only rough ideas and I didn't check their implication with each control. If the chosen idea/solution does not support all current controls, it's bad.
Implementation
I don't really want to specify the actual implementation due to the considerations above. There's just too many of them to do this beforehand. I just want to point out, that this task requires thorough preparation and consideration of separation of concerns (Soc) is key.
My basic assumption is that there's:
ipl\Web\Control\ViewModeSwitcher: A copy of\Icinga\Module\Icingadb\Web\Control\ViewModeSwitcher, more or lessipl\Web\Common\Controls: A trait providingcreateViewModeSwitcher(…). I've left out the method signature on purpose, as this is something that cannot be specified right now