RoukePouw commented 3 years ago

332

Extend makeAction function with javascript callback Set intermediate progress updates in xml Lock and unload browser page with javascript.

matthijskooijman commented 3 years ago

Also, be sure to put "Fixes: #332" in the commit message that actually implements the core of this PR, so that issue can be autoclosed.

RoukePouw commented 3 years ago

There is a lot of inline JS. I wonder if that would be better in a script tag, which can be done using the HTMLDocument writeScript() method.

Now sure how that would work, can you explain?

In other cases I would define a file with the functions that would get packed together into a minified library or something. But with hypha it seems to be the convention to group code together in a single file.

RoukePouw commented 3 years ago

This should be split over multiple commits.

How would you like this done?

matthijskooijman commented 3 years ago

How would you like this done?

Do you mean "Through what process"? Or "split into which commits"?

RoukePouw commented 3 years ago

Do you mean "Through what process"? Or "split into which commits"?

First in which commits you would like it split. (and why :-) so I can understand the thoughts behind it ) Secondly, yes, if you have pointers on how to do this, otherwise I'll do some DuckDuckGo-ing myself ;-)

matthijskooijman commented 3 years ago

I would make commits to:

Remove the extra request headers
Support callback for makeAction
Fix for str_replace(['"'], ['\''], $action); if you find it
Optionally the CSS for the messagebox, because it is fairly general (but probably better to just include this in the next commit)
Everything else (i.e. implementing the progress counter)

As for the how: In this case I think none of the current commits are usable as is, so it is probably best to do a (soft) reset back to the original commit (just before your first commit), to effectively remove all commits, but leave the changes in your working copy, so you can use e.g. git add -p to create new commits from that. If you would want to preserve existing commits but split or rearrange some, then using an interactive rebase would be the way to go.

RoukePouw commented 3 years ago

Commits have been split :)

matthijskooijman commented 3 years ago

I made a few more changes:

Added "mailinglist: " prefix to commit message
Converted a few more 'progress' to self::FIELD_NAME_PROGRESS
Converted a few other strings to constants too (as a separate commit) and moved one conversion of 'status' to this new commit.
Fixed one incorrect type annotation I noticed (was already broken in master).
Rebased on top of master.

With that, I'll merge this.

PlanBCode / hypha

Display progress for mailinglist send #357

332