redmica / redmica_ui_extension

This plugin adds useful UI improvements to RedMica.
GNU General Public License v2.0
41 stars 12 forks source link

Duplicate onchage events fire #17

Closed onozaty closed 2 years ago

onozaty commented 3 years ago

When changing the tracker in the new issue screen, the onchange event is fired in duplicate, causing multiple requests to be sent.

dupicate

There may be few things that are troubling with this. However, in View customize, this causes the script to run in duplicate.

In searchable_selectbox.js, the onchange event was being fired.

https://github.com/redmica/redmica_ui_extension/blob/ed3b0178e19265d1d30cc6617120bcdcde547c7f/assets/searchable_selectbox/javascripts/searchable_selectbox.js#L61-L64

https://github.com/redmica/redmica_ui_extension/blob/ed3b0178e19265d1d30cc6617120bcdcde547c7f/assets/searchable_selectbox/javascripts/searchable_selectbox.js#L69-L72

When this is commented out, the onchange event is no longer fired in duplicate. However, since I didn't understand the intent of this code, I couldn't decide if it was something I should remove.

Could you please confirm this?

The environment is as follows.

Environment:
  Redmine version                4.2.2.stable.21129
  Ruby version                   2.5.8-p224 (2020-03-31) [x86_64-linux]
  Rails version                  5.2.6
  Environment                    production
  Database adapter               PostgreSQL
  Mailer queue                   ActiveJob::QueueAdapters::AsyncAdapter
  Mailer delivery                smtp
SCM:
  Subversion                     1.7.14
  Git                            1.8.3.1
  Filesystem                     
Redmine plugins:
  redmica_ui_extension           0.0.1
ishikawa999 commented 3 years ago

@onozaty Thank you for sharing the issue. It is not common to have two change events triggered by a single change, and it is likely to cause problems when using the view customize plugin, so I will try to improve it.

I added this code because there was a problem with the change event not firing properly when some searchable select boxes were selected. I'm going to check if the problem is still reproduced now. If it is reproducible, look for another way to improve it. If not, delete the corresponding code.

ishikawa999 commented 3 years ago

I was able to confirm that removing the Rails.fire($(this)[0], 'change') code worked fine, so I removed it.

onozaty commented 3 years ago

Thank you very much!

iquiw commented 2 years ago

The change looks to make Redmine Issue Templates Plugin incompatible with this plugin.

When selecting a template in new issue page, template title and body are not inserted.

Issue Templates plugin uses native onchange event by addEventListener('change', ...). If they could be changed to jQuery on('change', ...), the problem will not happen.

Ref. Standard (native) JS events are no longer emitted in v4 #1908.

ishikawa999 commented 2 years ago

The problem in https://github.com/redmica/redmica_ui_extension/issues/17#issuecomment-922279390 seems to have a larger impact, so I will temporarily undo the changes I made. #19 I'll see if there's a way to solve both problems.

ishikawa999 commented 2 years ago

The change in https://github.com/redmica/redmica_ui_extension/pull/30 makes it so that only elements with some native change events will fire the change event twice, and the rest only once. It doesn't fix the underlying problem, but it will ensure that the change event will only fire once for most select boxes, and issues_template plugin will also work well. If you find a problem that is not solved by this change, please let us know.

Thanks to @onozaty for reporting this issue and to @iquiw for letting me know about the root cause of the problem.