-
-
Notifications
You must be signed in to change notification settings - Fork 552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
XWIKI-22191: Account disabling and password changing action links should be buttons #3148
base: master
Are you sure you want to change the base?
Changes from 3 commits
06051b7
7dcb59e
617518d
d043470
86dfba4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -340,8 +340,8 @@ | |
<div class="passwordManagement"> | ||
<h2>$escapetool.xml($services.localization.render('platform.core.profile.section.security'))</h2> | ||
<span class="buttonwrapper"> | ||
<a id="changePassword" href="$doc.getURL('view', 'xpage=passwd')">$escapetool.xml( | ||
$services.localization.render('platform.core.profile.changePassword'))</a> | ||
<button id="changePassword" onclick="location='$doc.getURL('view', 'xpage=passwd')'">$escapetool.xml( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmmm it means it won't work anymore when javascript is disabled. Not a big fan of this onclick attribute to be honest. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverted those changes in d043470 👍 |
||
$services.localization.render('platform.core.profile.changePassword'))</button> | ||
</span> | ||
</div> | ||
</div> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this change, you didn't introduced a button class here, but you replaced a link to a button, so the selector should be just button no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When checking out the displays of "button like links", I stumbled upon this one. It gets underlined on hover but there's no need for it to be underlined. See the third item of the PR description in the opening message.