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
The javascript api descriped in https://www.drupal.org/node/336122 has not been portet to the 8.x version of the flags module.
Proposed resolution
Refactor the 7.x api for drupal 8.x
User interface changes
None
API changes
Add API like in 7.x
Comment | File | Size | Author |
---|---|---|---|
#17 | interdiff-2842386-15-17.txt | 2.77 KB | martin107 |
#17 | 2842386-17.patch | 10.87 KB | martin107 |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedLaubi created an issue. See original summary.
Comment #2
joachim CreditAttribution: joachim as a volunteer commentedThanks for reporting!
Comment #3
martin107 CreditAttribution: martin107 as a volunteer commentedTo my mind
inserting before and after pseudo events, is a good idea
I am postponing until we have a ajax command associated with ajax flag actions.
In the issue I have linked to, its current name is ActionLinkFlashCommand, but from this issue I can see that we need to be make that name more generic.
Comment #4
martin107 CreditAttribution: martin107 as a volunteer commentedWe have that command now.
Comment #5
martin107 CreditAttribution: martin107 as a volunteer commentedHere is the code that adds before and after events for third party modules to bind their event listeners to.
I have included a 'working' test modules that adds the appropriate event listeners.
If you copy "flag_flash_event_test" out of the test directory into the custom module directory then you will see it adds a flag which attaches to any article that you care to create .. clicking on the new flag will cause the third party module to respond to before and after events. It dumps messages onto the console.
The next steps are to write a functional javascript test.. but I am inserting a pause here to show my working out .. for people to criticise. --- When I am sure the architecture is ok I will write the test
Comment #7
socketwench CreditAttribution: socketwench as a volunteer commentedTest failures unrelated.
Comment #8
socketwench CreditAttribution: socketwench as a volunteer commented...of a successful command respoonse
Small typo there.
Also needs tests, but I'm liking this.
Comment #9
martin107 CreditAttribution: martin107 as a volunteer commentedFrom #8 "small typo" ... thanks .. can't think why I was thinking of spoons while i mistyped that!
1) I am trying to shadow the functionality provided by the drupal7 version of this module.. and I see after a little review that I had missed a little bit of piping. In this iteration the response object is now plumbed down into the customEvents
2) To nail thing down in testing I am vandalizing the page by adding two paragraphs elements one for each event. I have found a way of locking down the test so they fail if the response object is not included as a detail object in the event.
3) With regard to namespacing, others may extend ActionLinkFlashCommand so in a couple of places I have made the name of things more unique,
for example
flagBefore becomes flagFlashBefore.
Comment #11
martin107 CreditAttribution: martin107 as a volunteer commentedOk so the failures in #9 are caused by our test instability
#2945195: test instability ( round 2 )
For which we I think we now have a solution ....
https://www.drupal.org/project/flag/issues/2945195#comment-12530969
So here I am retesting.
Comment #12
socketwench CreditAttribution: socketwench as a volunteer commentedRetesting now that the above issue is in.
Comment #14
socketwench CreditAttribution: socketwench as a volunteer commentedComment #15
martin107 CreditAttribution: martin107 as a volunteer commentedComment #17
martin107 CreditAttribution: martin107 as a volunteer commentedI have not fixed the test ... yet
but there are enough pieces of the solution here to be worth talking about.
The problem when debugging is that the js files provided by the mock third party module is not on the page.
ie event_listener.js - I have no solution to that yet.
Here are the changes ...
a) flag_flash_event_test/config/install/flag.flag.event_test.yml was creating when I had a different test stratergy
it is no longer needed.
b) The code below cannot tolerate white spaces.
c) I have made flag_flash_event_test.info.yml follow the pattern set in flag_test_plugin.info.yml
Comment #19
martin107 CreditAttribution: martin107 as a volunteer commentedSo when I described in #17 ...
"Why is the javascript added in reality, in manual testing, but consistently screws up in test?"
Well this core issue is similar but not exactly that ....
#1988968: Drupal.ajax does not guarantee that "add new JS file to page" commands have finished before calling said JS
I am going to postpone this and work on that ...