umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.45k stars 2.68k forks source link

v8: Custom grid editor control.active is gone #7332

Closed iOvergaard closed 4 years ago

iOvergaard commented 4 years ago

This issue is concerning working with custom grid editors in the backoffice UI where you can no longer see if a grid editor is active in its own controller.

Umbraco version

I am seeing this issue on Umbraco version: 8.4.0

Reproduction

Bug summary

In Umbraco 7, you could know if a grid editor was active by checking for something like:

if ($scope.control.active) { ... }

And by "active" I mean that a user has clicked on a grid editor inside the grid.

The backoffice UI would also itself show a class on the grid editor in grid.html by checking for control.active which would indicate that there existed a boolean property on the control named "active":

ng-class="{'-active': control.active}"

In Umbraco 8 this property seems to have moved to the controller Umbraco.PropertyEditors.GridController and is being used in grid.html like this:

ng-class="{'-active': control === active}"

Thus it is no longer possible to check for $scope.control.active in the controller of the grid editor, i.e. this no longer works:

<div class="umb-grid-textarea" ng-if="control.active">I am active!</div>

Cause

I am not completely certain, but I found the function clickControl has changed in the aforementioned controller and as you may notice, the property on the control is no longer toggled:

Umbraco 7.15.3:

$scope.clickControl = function (index, controls, cell) {
  controls[index].active = true; // Property toggled here
  cell.hasActiveChild = true;
};

Umbraco 8.4.0:

$scope.clickControl = function (index, controls, cell, $event) {
    $scope.active = controls[index]; // Controller property here instead
    $scope.currentCellWithActiveChild = cell;
    $event.stopPropagation();
};

Actual result

I would expect to be able to see if a control has been activated. Preferably inside the $scope.control method to support backwards compatibility with Umbraco 7.

nul800sebastiaan commented 4 years ago

Hey there @Jacob87 - for v8 we made many changes to the grid, so I'm sure this was part of a general refactor. We didn't strive to be backwards compatible with v7.

My main question is: how are you using the active state and why do you need it, what is the big benefit for you knowing which element in the grid is being edited right now?

iOvergaard commented 4 years ago

Hi @nul800sebastiaan Thanks for responding! The main benefit is that I can choose to show extra toggles when an editor is being edited right now. I can show a settings cog wheel, for instance, or I can swap out a placeholder image with a code editor, and just in general make fields editable only when the editor is active.

Of course I can activate a dialog when clicking on a grid editor and show some of these things inside that dialog instead, but I feel that what gives the best editor experience is to know when a control is being edited so I can activate some "helpers" as mentioned before.

abjerner commented 4 years ago

Hi @Jacob87,

I think the active property was initially introduced for the grid to keep track of the active state internally, but but probably not intended for use in custom code.

I'm not sure about the complexity of your code, but could using the -active CSS class be a suitable alternative to the property? Eg. something similar to the LESS/SASS snippet below:

.umb-control {
  .my-control .a-feature {
    display: none;
  }
  &.-active {
    .my-control .a-feature {
      display: block;
    }
  }
}
iOvergaard commented 4 years ago

Hi @abjerner

I believe you are right, but as for myself I discovered the property while reading through published examples of custom grid editors on various fora, so the property is somewhat known in the community.

As for using the -active class in a CSS, that's a good idea and should work well when you just want to display extra buttons and the like in a grid editor, and I have been using that in lieu of the Angular property where possible.

The whole thing is merely concerning upgrading from Umbraco 7 to 8, when I have to rewrite grid editors where it could look like something was missing or taken out unintentionally. It seemed like an easy property to put back.

nul800sebastiaan commented 4 years ago

Thanks for chipping in with the feedback Anders! Some breaking changes were to be expected going form v7 to v8. It's been a while since v8 has been out and people have not seemed to miss $scope.control.active just yet. I'm sure it was removed to clean up all the things that were never really used.

It's a shame that at least one person was actively using this, maybe there's more to come along later and at that point we can consider adding it back. For now, I don't see a very compelling reason to add this back. I'll close the issue and we'll see if it gets more traction later on.

Thanks though, for the discussion @Jacob87! Always good to learn how people are using Umbraco in ways we didn't know about, I'll send some feedback about this usage pattern to the team! 👍

johnsharp1975 commented 3 years ago

I found this feature to be very useful. I can see the way to do things using the CSS -active class but having an actual .active field makes the code feel correct.

Yes, I know it's been over a year since the ticket was closed.

I did work out that if I work my way up the $parent tree far enough I can get to $parent.$parent....$.parent.active. I wondered what that object was and if it had a significant name so that I may be able to do something like:

vm.parentController = $scope.<SPECIALPARENTNAME>;
vm.active = function () => vm.parentController.active;
johnsharp1975 commented 3 years ago

Oh, in terms of why this is useful, I used it to kind of show what the control might resemble when it's not active and have fields displayed that can be edited when it's active.

johnsharp1975 commented 3 years ago

In the event anyone else stumbles on this post I found a few ways to overcome this change; the easiest of the ways I got to work was:

    .controller("controller", function ($scope) {

        $scope.gridScope = angular.element(document.getElementById('umb-grid')).scope();
        $scope.isActive = function () {
            return $scope.gridScope.active === this.control;
        }

this allows ng-show="isActive()"

HIH someone

iOvergaard commented 3 years ago

In the event anyone else stumbles on this post I found a few ways to overcome this change; the easiest of the ways I got to work was:

That is truely a magnificient Angular hack, @johnsharp1975 . Thanks for posting this “solution” 👍

I also find your description of why this property is useful very compelling. As it looks like the Grid editor may be replaced by something like the new Block list editor at some point, I fear that this property will never be added back in, but your sentiment just goes to show how important it is to consider how you, as a developer, is able to make a really great content editor experience in the backoffice.

johnsharp1975 commented 3 years ago

I'm glad you like it :-) I thought I'd broken something when I upgraded from 7.x to a more up to date version recently. I guess with this fix it means that any future option should be handled too.

henryson commented 2 years ago

I spent some time trying to find out why my control.active did not work as expected. Glad I finally googled it and found this thread. We have a project in U7 with a lot of grid editors and each one uses control.active to toggle between preview and editing and we expect to upgrade to U10 this fall. Good to know there is a workaround at least.