This commit includes more changes, all them also adding the :void
return type to unit tests missing them.
The difference is that all these changes, while also detected
perfectly by the moodle.PHPUnit.TestReturnType sniff, were not
auto-fixed (like the previous commit ones), because all them
do include some "return" statement and, for safety, we don't
fix them.
All the cases have been visually inspected and confirmed that
the existing "return" statements always belong to anon
functions within the test body and not the test own return statement.
While this change is not 100% required now, it's good habit
and we are checking for it since Moodle 4.4.
All the changes in this commit have been applied automatically
using the moodle.PHPUnit.TestReturnType sniff and are, exclusively
adding the ": void" return types when missing.
There is a difference in the behaviour of the calendar block when it is on the calendar page and when it is not.
On the calendar page, when the user clicks on the date or link next/previous month or day name in the calendar block,
it will have the effect of changing the URL, which should not happen.
The patch also includes hiding popover after the user clicks the day number. When the user clicks on the date or
is focused on the date and then presses enter on the keyboard, the popover does not automatically close.
To fix this, I added an event type, "click", to be attached to the hidePopover function and added conditioning to
the target element when there was a "click" event.
An additional step was added to the Behat calendar for the click event to ease the testing process.
For the popover, I set the "animation" to false to avoid the random Behat failure caused by the slow animation.
The TinyMCE.remove() function is killing other events on Firefox
We need to modify the setupForElementId() to make TinyMCE to
remove itself outside of the event loop
- Replace .form-group Boostrap helper class with .mb-3. The .form-group class was only
adding margin bottom styles, so it is an straightforward change.
- Replace .form-group references in SCSS files with .fitem now .form-group has been removed.
- There were some other .form-group occurrences in the code that were using it not for styling
but incorrectly for managing some logic. These have been also replaced with .fitem or removed.
This has been generated running the following Sniff,
part of the Moodle's CodeSniffer standard:
- PSR2.Methods.MethodDeclaration
It just ensures all the function declarations have
the correct order for:
- abstract and final.
- visibility (public, protected, private).
- static.
So, all the lines modified by this commit are function declarations
and the only changes are in the positions of those keywords.
This has been generated running the following Sniffs, all
them part of the Moodle's CodeSniffer standard:
- PSR12.Functions.ReturnTypeDeclaration
- PSR12.Functions.NullableTypeDeclaration
- moodle.Methods.MethodDeclarationSpacing
- Squiz.Whitespace.ScopeKeywordSpacing
All them are, exclusively, about correct spacing, so the changes
are, all them, only white space changes.
Only exceptions to the above are 3 changes what were setting the
return type in a new line, and, when that happens, the closing
parenthesis (bracket) has to go to the same line than the colon.
The UNION caused the query to be run in a way that is very
inefficient on MySQL, separating the queries causes each of them to
run in a much more efficient form.
Any duplicated will be filtered out on the PHP side instead of in
the database. On large Moodle sites this is preferable as the
extract is likely to be performed on a server dedicated to running
the Moodle cron and so there will be less of effect on resources
that are used to serve end users.
* calendar_get_link_previous() and calendar_get_link_next() are no
longer used anywhere except from the already deprecated
calendar_top_controls() so we should deprecate these as well.
