[4.1] Align the inline help button to the rest of the btns #37356
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I have tested this item ✅ successfully on 50c0643 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37356. |
Quy
reviewed
Mar 23, 2022
Quy
reviewed
Mar 23, 2022
|
I have tested this item ✅ successfully on 50c0643 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37356. |
|
Thx |
roland-d
pushed a commit
that referenced
this pull request
Jun 4, 2022
) * Captive TFA Import YubiKey plugin * Captive TFA Prepare SQL for new plugins * Captive TFA Import Fixed plugin (EXAMPLE) * Captive TFA System plugin * Captive TFA Replace the two factor authentication integration in the core * Captive TFA Fix wrong SQL / table name * Captive TFA Use correct prefix in the TFA helper when getting config UI * Captive TFA Fix a whoopsie or four * Captive TFA Coffee has long stopped working * Captive TFA Format the Methods page * Captive TFA Fix wrong TFA method internal name * Captive TFA Make sure we get the right view in the controllers * Captive TFA Remove yet another integration of the legacy TFA * Captive TFA Automatic migration from old TFA upon first login * Captive TFA Frontend MVC * Captive TFA Frontend routing * Captive TFA Style the method select page * Captive TFA Missed a legacy integration which needs removal * Captive TFA Better format of the configuration UI in the profile page * Captive TFA Use language strings when migrating data from legacy TFA * Captive TFA Only show the prompt to add a TFA method if none is already added * Captive TFA YubiKey should allow entry batching This means that you can authenticate with any registered YubiKey in your user profile. * Captive TFA Replace Tfa::triggerEvent * Captive TFA Import WebAuthn plugin * Captive TFA Improve TFA behavior on non-HTML pages. Basically, block them! * Captive TFA Replace alerts with Joomla messages * Captive TFA Move onUserAfterDelete code to the `joomla` user plugin * Captive TFA Remove the System - Two Factor Authentication plugin Use a trait for the application and fold the rest of the code into Joomla's core user plugin. * Captive TFA Remove accidental leftover references to loginguard * Captive TFA Import Code by Email plugin * Captive TFA Post-installation messages * Captive TFA Enable the TFA plugins on NEW installations * Captive TFA XML formatting * Captive TFA Language and grammar in comments * Captive TFA Rearrange XML attributes * Captive TFA Fix typo * Captive TFA Fix wrong language key name * Captive TFA Remove leftover legacy TFA options * Captive TFA Fix wrong CSS class * Captive TFA Merge the padding classes * Captive TFA This lang string should never have had a link * Captive TFA Hide the Key emoji from screen readers * Captive TFA Accessibility improvements * Captive TFA Accessibility improvements * Captive TFA Accessibility improvements * Captive TFA Accessibility improvements * Captive TFA Accessibility improvements * Captive TFA Accessibility improvements * Captive TFA Use “Two Factor Authentication” / TFA consistently * Captive TFA Tytytytypo * Captive TFA Fixed PHPCS issue unrelated to PR but reported by Drone nonetheless * Captive TFA Lang improvement Co-authored-by: Brian Teeman <brian@teeman.net> * Captive TFA Lang improvement Co-authored-by: Brian Teeman <brian@teeman.net> * Captive TFA Remove no longer valid plugin options * Captive TFA Typo in plugin path * Captive TFA Move TFA options in com_users config next to the password options * Captive TFA Add Show Inline Help button to com_users' options page * Captive TFA Move loading static assets to the view template See #37356 for the reasoning. This should REALLY have been documented somewhere... * Captive TFA Fixed wrong plugin path * Captive TFA Language style guide Co-authored-by: Brian Teeman <brian@teeman.net> * Captive TFA Language style guide * SQL code style and consistency fixes * Add "CAN FAIL" installer hint * Change longtext to mediumtext * Change longtext to mediumtext in update script * No default value for method * Use real null values for last_used * Captive TFA Fix JS linter errors * Captive TFA Fix PHPCS issues after merging @richard67 's PR * Captive TFA Update formatRelative to use JNEVER, simplifying the code in the view templates. * Captive TFA Fix typo * Captive TFA Fix transcription error * Captive TFA Show correct TFA column in the backend Users page * Captive TFA Fix PHPCS errors in UsersModel unrelated to this PR * Captive TFA Add note about supported browsers in TOTP's link * Captive TFA Remove bogus ESLint notice about qrcode * Captive TFA Fix confusing prompt * Captive TFA Consistently change ->qn to ->quoteName * Captive TFA Strict equality check * Captive TFA Move setSiteTemplateStyle to the views * Captive TFA Rename regbackupcodes to regenerateBackupCodes * Captive TFA Rename dontshowthisagain to doNotShowThisAgain * Captive TFA Throw deprecated notices from deprecated methods * Captive TFA Strict comparison * Captive TFA Typo in comment * Captive TFA Rename TwoFactorAuthenticationAware to TwoFactorAuthenticationHandler * Captive TFA Fix comment typo * Captive TFA Remove variables from SQL when not necessary * codestyle changes * Renamed SiteTemplateAware to SiteTemplateTrait Change made against feedback item #37811 (review) for pull request #37811 with title "Improved Two Factor Authentication". Feedback item marked the SiteTemplateAware trait name and had the following content: > Still ...Aware is not a good name for a trait, since it usually denotes interfaces * Remove more instances of "2SV" Per feedback item #37811 (comment) * s/Two Step Verification/Two Step Validation/ Per feedback item #37811 (comment) * Language style Per feedback item #37811 (comment) * Remove unnecessary language string * Remove redundant paragraph tags from PLG_TWOFACTORAUTH_EMAIL_XML_DESCRIPTION Per feedback item #37811 (comment) * Remove redundant paragraph tags from PLG_TWOFACTORAUTH_EMAIL_XML_DESCRIPTION Per feedback item #37811 (comment) The other file with the same language string I forgot to put in the previous commit. * Remove the info tooltip in the methods list Addresses feedback in #37811 (comment) * Simplify the TFA enabled / disabled message Per feedback item #37811 (comment) * Fix layout of backup codes in methods list Per feedback item #37811 (comment) * Fix mail message Per #37811 (comment) * Confirm TFA method deletion Per feedback item #37811 (comment) * Simplify code label in Email plugin Per feedback #37811 (comment) We show short instructions above the field and the field label is simplified. Applied the same change to the Fixed plugin for consistency. * Remove more dead code referencing the legacy TFA * Use concrete events This was the plan all along. Now that #36578 is merged we can FINALLY do it! * WebAuthn support for some Android devices and FIDO keys Backported from #37675 * Rename Tfa to Mfa Ongoing process * Move Joomla\CMS\Event\TwoFactor to Joomla\CMS\Event\MultiFactor Ongoing process * Two Factor Authentication => Multi-factor Authentication Ongoing process * `#__user_tfa` => `#__user_mfa` Ongoing process * twofactorauth => multifactorauth Ongoing process * Change the post-install message Ongoing process * Remove references to “second factor” Ongoing process * Remove the legacy TFA plugins * I missed a few things * I missed a few more things * Wrong redirection from post-installation messages Addresses #37811 (comment) * Fix NotifyActionLog expected event names Addresses feedback item #37811 (comment) * Improve display of Last Used date Addresses feedback item #37811 (comment) * MFA extension helper moves the group to the correct alpha order in the array now that it doesnt begin with T * Remove unused field * Remove no longer used language strings * Undo changes in old SQL scripts * Improve layout and accessibility of the methods list page Based on VoiceOver testing on macOS 12.4 and the feedback from #37811 (comment) and #37811 (comment) * Add missing options to plg_multifactorauth_email * Sort lines alphabetically Why not confuse the translators with out of order labels providing zero context to what they are translating? It's the One True Joomla Way... * Add label to the One Time Emergency Password input Per feedback item #37811 (comment) * Sort lines * Fix PHPCS complaint * Formatting of XML files * Forgot to remove extra CSS class * Apply suggestions from code review Formatting, wrong copyright and version information Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Update build/media_source/plg_multifactorauth_webauthn/js/webauthn.es6.js Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Fix update SQL Feedback item #37811 (review) * Onboarding would result in a PHP exception Feedback item #37811 (comment) * Make MFA plugins' publish state consistent between MySQL and PostgreSQL Feedback item #37811 (review) * Update administrator/components/com_users/src/Controller/MethodsController.php Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Update administrator/components/com_admin/sql/updates/mysql/4.2.0-2022-05-15.sql Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Update administrator/components/com_admin/sql/updates/postgresql/4.2.0-2022-05-15.sql Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Update administrator/components/com_admin/sql/updates/mysql/4.2.0-2022-05-15.sql Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Update administrator/components/com_admin/sql/updates/postgresql/4.2.0-2022-05-15.sql Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Update administrator/components/com_admin/sql/updates/postgresql/4.2.0-2022-05-15.sql Co-authored-by: Richard Fath <richard67@users.noreply.github.com> * Restore obsolete language strings Per discussion with @bembelimen I had to rename One Time Emergency Passwords to Backup Codes so as not to make major changes to the obsolete language strings. Having them named One Time Emergency Passwords (OTEPs) was both misleading (they are not passwords, they are second factor authentication codes) and would collide with the `_OTEP_` component of language existing strings. Backup Codes is a good compromise, one that is also field tested for nearly seven years. So, there you go! * Re-add the obsolete plugins' language files Per discussion with @bembelimen Yes, it's pointless, it looks wrong, it is what it is. At least I've put a header that this file needs to be removed. * Remove no longer used twofactor field * Rename CSS class to com-users-profile__multifactor * Update administrator/language/en-GB/plg_multifactorauth_email.sys.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Update administrator/language/en-GB/plg_multifactorauth_email.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Update administrator/language/en-GB/plg_multifactorauth_email.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Update administrator/language/en-GB/com_users.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Update administrator/language/en-GB/com_users.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Update administrator/language/en-GB/com_users.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Update administrator/language/en-GB/com_users.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Update administrator/language/en-GB/com_users.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Accessibility improvement * Improve language * Change the heading level * Fix case of extension registry file Regression after renaming TFA to MFA * Remove accidental double space after echo * Remove BS3 leftovers * Remove BS3 leftovers * Remove BS3 leftovers * Update administrator/components/com_users/tmpl/methods/list.php Co-authored-by: Brian Teeman <brian@teeman.net> * Update components/com_users/tmpl/methods/list.php Co-authored-by: Brian Teeman <brian@teeman.net> * PHP warnings when there are no MFA plugins enabled * MFA onboarding was shown with no MFA plugins enabled * Backup codes alert is narrower than page on super-wide screens * Backup codes alert heading font size fix in backend * Revert wording for JENFORCE_2FA_REDIRECT_MESSAGE * Backend users without `core.manage` on com_users were blocked They were blocked from setting up / manage their on MFA, blocked from the onboarding page and blocked from the captive login page. * Onboarding in backend shouldn't have a Back button * Improve layout of method add/edit page * Remove unnecessary H5 tag from TOTP setup table * Kill that bloody Back button with fire * MFA WebAuthn: use Joomla.Text instead of Joomla.JText * MFA WebAuthn: show meaningful error on HTTP * MFA Email: more sensible email body * MFA WebAuthn: must be able to edit the title * MFA add/edit: remove placeholders, replace with help text * Heading levels We assume an H1 will already be output on the page. This is always true on Atum and never true on Cassiopeia — but very likely on real world sites's frontend templates. So it's a compromise which is at least better than the previous case of starting at h3 or h4. * Editing a user would show the wrong interface When editing a user other than ourselves we need to show the MFA editing interface for the user being edited, not the MFA editing interface for our own user. * Refactor security checks Now they are conforming to the original intention * Add missing Group By to the SQL query * Show MFA enabled when a legacy method is enabled * Users: filter by MFA status * Language clarification * Move the frontend onboarding page header to the top * User Options language clarification * PostgreSQL installation SQL wasn't updated * Adding periods to the end of lines of error messages you will never, ever see * Remove a tab * Remove another tab from a comment * Typo removing junk * Remove useless imports * Busywork * Typo in the INI file * Align comment * Remove redundant SQL for PostgreSQL * Typo in labels' `for` attribute * Move backup codes to the top of the page * Mandatory and forbidden MFA was not taken into account If only one group matched, due to typo. * Show information when MFA is mandatory * Make the buttons smaller * The secondary button looks horrid in the frontend * Redirect users to login page in the frontend When they try to access a captive or methods / method page. * MFA Email: fallback to standard mailer when the mail template isn't installed * Delete backup codes when the last MFA method is deleted * Use text inputs for TOTP With the correct input box attributes * Fix the buttons for WebAuthn * Clarify language strings * Use toolbar buttons in the backend Except for screen size small and extra small. Over there we ALSO display the inline content buttons because the toolbar buttons are hidden behind an unintuitive gears icon. JUST BECAUSE THE DEFAULT JOOMLA WAY IS TO USE A TOOLBAR IT DOES NOT MEAN THAT IT MAKES SENSE ALWAYS, EVERYWHERE. THE USER IS KING. WE SERVE THE USER, NOT OURSELVES! * Change the icon classes * Forgot to copy over the changes to the frontend * Regression: configure existing authenticators We used to set field_type to custom to make the code entry disappear. After the changes to the field type handling we need to instead set input_type to hidden. * Backup codes should never become the default method automatically * Improve methods list layout Now it is more clear which methods are enabled and which are available. * Use toolbar buttons in backend pages Except when the screen size is extra small which is the point where the toolbar is hidden and the interface becomes unintuitive. * Fix return URLs for backend MFA edit pages * Edit / Delete buttons mention the auth method name in the respective button's visually hidden text * RTL aware back buttons * Consistent use of the term Fixed Code * Fix typo Co-authored-by: Brian Teeman <brian@teeman.net> Co-authored-by: Richard Fath <richard67@users.noreply.github.com> Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request for Issue # .
Summary of Changes
The static assets should only be added inside a layout. The inlinehelp toolbar button is adding the assets (a js file) inside the logic and this makes it impossible to override/remove the script in an easy fashion (eg by editing the relative layout). Also it's pretty obvious that this is wrong as it's the only button that doesn't adhere to the CMS' norm (add the assets only inside the layouts).
Testing Instructions
Apply the PR and check that the inlinehelp still works as before (check the global options)
Actual result BEFORE applying this Pull Request
Expected result AFTER applying this Pull Request
No assets added in the logic
All the assets added in the layouts
Documentation Changes Required
No, but it would be good to pull a flag whenever a PR is adding static assets in places other than the layouts