Moodle

convert chat to YUI3 and new coding style - test and fix regressions

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 2.0
  • Fix Version/s: 2.0
  • Component/s: Chat
  • Labels:
    None
  • Affected Branches:
    MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Activity

Hide
Sam Hemelryk added a comment -

All YUI3 now + tested, still not the nicest module but major improvements will probably come as of 2.1.
Let me know if you spot any problems Petr,

Cheers
Sam

Show
Sam Hemelryk added a comment - All YUI3 now + tested, still not the nicest module but major improvements will probably come as of 2.1. Let me know if you spot any problems Petr, Cheers Sam
Hide
Petr Škoda (skodak) added a comment -

reopening:
1/ M.util.in_array() not in cvs yet (committed)
2/ the gui_header_js is mixing three different inits in one huge class which is extremely confusing, going to split it into 3 separate init functions with standard coding style
3/ gui_ajax - there is incorrect new function() { handler code } in the mouse events, also I do not like the YUI.add() stuff much (the same as in 2/)

I personally think the YUI.add() is worse than real external YUI modules because:

  • the requires at the bottom of YUI.add does not work and has to be duplicated in the standard PHP $module->requires
  • there are some changes planned in YUI 3.1.x that may significantly change how loading of YUI modules works, I took that into consideration when designing the ->js_init_call() and ->js_module()
  • the Y is duplicated - first in YUI.add callback and then in js_init_call which is confusing

In general the mod/chat/ is not the best example how to write modular modules because the chat interfaces are not real plugins, going to note this both in PHP and JS code.

I was kind of surprised to see the "themes" in the code, I think this duplication should be removed or at least it should by renamed to something else, not theme.

There is another pending problem in chatd.php because it needs the allow_call_time_pass_reference which is deprecated in 5.3 and will be remove in PHP 6. In theory it might be possible to make the ajax chat fast enough and remove the chat deamon+sockets client+gui_header...... (probably Moodle 2.1 target)

Committing my refactored code, please close after review, feel free to propose any improvements...

Show
Petr Škoda (skodak) added a comment - reopening: 1/ M.util.in_array() not in cvs yet (committed) 2/ the gui_header_js is mixing three different inits in one huge class which is extremely confusing, going to split it into 3 separate init functions with standard coding style 3/ gui_ajax - there is incorrect new function() { handler code } in the mouse events, also I do not like the YUI.add() stuff much (the same as in 2/) I personally think the YUI.add() is worse than real external YUI modules because:
  • the requires at the bottom of YUI.add does not work and has to be duplicated in the standard PHP $module->requires
  • there are some changes planned in YUI 3.1.x that may significantly change how loading of YUI modules works, I took that into consideration when designing the ->js_init_call() and ->js_module()
  • the Y is duplicated - first in YUI.add callback and then in js_init_call which is confusing
In general the mod/chat/ is not the best example how to write modular modules because the chat interfaces are not real plugins, going to note this both in PHP and JS code. I was kind of surprised to see the "themes" in the code, I think this duplication should be removed or at least it should by renamed to something else, not theme. There is another pending problem in chatd.php because it needs the allow_call_time_pass_reference which is deprecated in 5.3 and will be remove in PHP 6. In theory it might be possible to make the ajax chat fast enough and remove the chat deamon+sockets client+gui_header...... (probably Moodle 2.1 target) Committing my refactored code, please close after review, feel free to propose any improvements...
Hide
Sam Hemelryk added a comment -

Hi Petr, everything looks fantastic
Thanks for explaining the YUI.add issue, wow will be nice when they fix that!
Cheers
Sam

Show
Sam Hemelryk added a comment - Hi Petr, everything looks fantastic Thanks for explaining the YUI.add issue, wow will be nice when they fix that! Cheers Sam

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: