Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
in views_ui/js/ajax.js
Drupal.AjaxCommands.prototype.viewsHighlight = function (ajax, response, status) {
$('.hilited').removeClass('hilited');
$(response.selector).addClass('hilited');
};
but no css styling is defined and 'hilited' is not inline with command name
Proposed resolution
1) Use "hilighted" class
2) add styling
Remaining tasks
tbd
User interface changes
proper highlighting of contend
API changes
no
Comment | File | Size | Author |
---|---|---|---|
#23 | Selection_973.png | 10.94 KB | longwave |
#15 | 2252715-15.patch | 3.61 KB | Lendude |
#15 | interdiff-2252715-11-15.txt | 537 bytes | Lendude |
Comments
Comment #9
LendudeI took a little dive into this, and completely unclear why we have this. Didn't dive deeper into the git blame then 2009 when Views 3 was created and this already existed. It is unclear to me what this is supposed to do, but I assume it should highlight the section of the Views UI that just got updated after adding/rearranging handlers.
This has zero test coverage and has been broken since before 8.0.0 was released, it's also broken in D7. My proposal would be to remove this.
Todo:
* Remove adding the Command from \Drupal\views_ui\Form\Ajax\ViewsFormBase::ajaxFormWrapper
* Remove \Drupal\views\Ajax\HighlightCommand
* Remove Drupal.AjaxCommands.viewsHighlight from core/modules/views_ui/js/ajax.es6.js
Comment #10
LendudeSo something like this....
Comment #11
LendudeOops something snuck into #10, ignore that one
Comment #12
longwavePatch fails to apply.
But +1 to removing this. I did a bit more archaeology and found that the "hilite" feature was first added to Views 6.x-2.x prior to alpha1 in 2008 in https://git.drupalcode.org/project/views/-/commit/999b2e5390c04fb8ef442e... and then the "hilited" CSS was removed in 2010 when the Views admin CSS was overhauled, and never added back again, but the JS still lives on.
Comment #13
andypostLooks removal is blocked on deprecation for plugins but ++ remove
I found no usages in contrib
Comment #14
LendudeAh yeah, rolled this against 9.2, not 8.9
Nice digging @longwave :)
Not sure if this needs to wait for plugin deprecations? It's an Ajax Command that we are taking out, couldn't we just follow the deprecation guidelines for classes if we want deprecations at all? Seems a bit silly really, it's just dead code, but ¯\_(ツ)_/¯
Comment #15
LendudeRemoved the unused use
Comment #16
longwaveThis is not used in contrib: http://grep.xnddx.ru/search?text=HighlightCommand&filename=
Previously we have just removed unused and entirely broken bits of code without going down the deprecation route, maybe we can just do that here?
Comment #17
catchI think this is fine to remove as dead and never-working code.
Comment #18
andypostAs everyone agree to remove as deadcode
Comment #19
alexpottWhat does worked mean in this context? As far as I can see the JS and Ajax class work as described. It's only that in core we don't have .hilited class defined and, importantly, #section is not used as a class name.
Yes this is probably safe to remove as someone relying on it is unlikely. But I'm not sure it is dead code - code that is unreachable and not working. I think we should consider deprecating the class and the JS and removing core usage and then removing in Drupal 10 - just in case someone is using this in custom code.
Comment #20
catchHmm #19 is a good point. We can remove the usage because markup changes are allowed, but given it's used that means it 'works' in the sense of being runnable code, so it should go through deprecation even though we're 99.9% sure no-one is relying on it.
Comment #21
catchBack to needs work.
Comment #22
longwaveI was also suspicious of this:
The only place #section is referred to in form state is ViewUI::getStandardButtons():
But then does that mean all the other
$form['#section']
stuff is redundant as well?Is this "mark changed" also referring to the highlighting feature?
Also, if we add some CSS rules for the class, does this actually do something useful in the Views UI that we might want to keep?
Comment #23
longwaveI added this CSS rule:
Then in Views UI if I click "Reorder displays", after closing the modal and reopening the menu, the background is highlighted in red:
This works for some, but by no means all, of the dropdowns in Views UI. For example it looks like it is supposed to work when rearranging filters:
But the class name here is entirely wrong and so the highlighting isn't applied properly.
I don't think this feature is very useful and it seems like we can deprecate this now and then remove all the
#section
related code in Drupal 10.Comment #24
andypost@longwave thank you for hint!
There's more usages in core