Skip to content
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-21597: Make the rights UI use icon themes #2649

Merged
merged 5 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@ require(['jquery'], function($) {

#admin-page-content label > input[type="checkbox"],
#admin-page-content label > input[type="radio"] {
vertical-align: bottom;
vertical-align: middle;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,30 +257,26 @@ $xwiki.ssfx.use('js/xwiki/table/livetable.css', true)
#if ("$!request.editor" == 'globaladmin' || "$!editor" == 'globaladmin')
#set ($auth_view = $targetDocument.getObject('XWiki.XWikiPreferences').getProperty('authenticate_view').getValue())
#if ("$!auth_view" == '1')
#set ($view_icon = $xwiki.getSkinFile('js/xwiki/usersandgroups/img/allow-black.png'))
#set ($view_alt = 'yes')
#set ($view_checked = 'checked')
#else
#set ($view_icon = $xwiki.getSkinFile('js/xwiki/usersandgroups/img/none.png'))
#set ($view_alt = 'no')
#set ($view_checked = '')
#end
<dt>
<label for="authenticate_view">
<input type="image" id="authenticate_view" alt="$view_alt" src="$view_icon" class="icon" />
<input type="checkbox" id="authenticate_view" $view_checked/>
$services.localization.render('authenticate_view')
</label>
<dd>#* A <dd> must be present after a <dt> to be HTML5 valid *#</dd>
</dt>
#set ($auth_edit = $targetDocument.getObject('XWiki.XWikiPreferences').getProperty('authenticate_edit').getValue())
#if ("$!auth_edit" == '1')
#set ($edit_icon = $xwiki.getSkinFile('js/xwiki/usersandgroups/img/allow-black.png'))
#set ($edit_alt = 'yes')
#set ($edit_checked = 'checked')
#else
#set ($edit_icon = $xwiki.getSkinFile('js/xwiki/usersandgroups/img/none.png'))
#set ($edit_alt = 'no')
#set ($edit_checked = '')
#end
<dt>
<label for="authenticate_edit">
<input type="image" id="authenticate_edit" alt="$edit_alt" src="$edit_icon" class="icon" />
<input type="checkbox" id="authenticate_edit" $edit_checked/>
$services.localization.render('authenticate_edit')
</label>
<dd>#* A <dd> must be present after a <dt> to be HTML5 valid *#</dd>
Expand All @@ -294,16 +290,13 @@ $xwiki.ssfx.use('js/xwiki/table/livetable.css', true)
#set ($guest_comment_requires_captcha = $xwiki.getSpacePreferenceAsInt('guest_comment_requires_captcha', 0))
#end
#if($guest_comment_requires_captcha == 1)
#set ($guest_comment_requires_captcha_icon = $xwiki.getSkinFile('js/xwiki/usersandgroups/img/allow-black.png'))
#set ($guest_comment_requires_captcha_alt = 'yes')
#set ($captcha_checked = 'checked')
#else
#set ($guest_comment_requires_captcha_icon = $xwiki.getSkinFile('js/xwiki/usersandgroups/img/none.png'))
#set ($guest_comment_requires_captcha_alt = 'no')
#set ($captcha_checked = '')
#end
<dt>
<label for="guest_comment_requires_captcha">
<input type="image" id="guest_comment_requires_captcha" alt="$guest_comment_requires_captcha_alt"
src="$guest_comment_requires_captcha_icon" class="icon" />
<input type="checkbox" id="guest_comment_requires_captcha" $captcha_checked/>
$services.localization.render('rightsmanager.guestcommentrequirescaptcha')
</label>
</dt>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,11 @@
width: 15% !important;
}

/* This button has no border or background, but wraps an image to allow keyboard navigation. */
#usersandgroupstable button.rights-edit {
border-color: transparent;
background: transparent;
#usersandgroupstable button.rights-edit.yes {
color: $theme.notificationSuccessColor;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels a bit weird here to use notification(Success|Error)Color, now maybe that's the best practice in themes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's some of the old colortheme variables. I'm not sure we have a best practice for this. I think I did use those old variables since others are used all throughout the file and I'd rather keep consistent inside one file.

To be honest this whole file looks quite old and breaks current CSS codestyles in multiple places.
I opened a topic on the forum about introducing a new type of label for codestyle, this file could really use a ticket :)

I just checked our HTML and CSS best practices and the entry don't hard code colors in properties - use [ColorTheme variables](https://extensions.xwiki.org/xwiki/bin/view/Extension/Color%20Theme%20Application#HUsingColorThemesvariables) points to a doc that uses the old colortheme variables. We might want to update the link or the doc.

I'll look a bit more into the state of deprecation of this old colortheme, it's not clear to me what it is yet. Using the old colortheme everywhere does not look like something we should recommend...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the doc to include a link to how to use the new variables.

}
#usersandgroupstable button.rights-edit.no {
color: $theme.notificationErrorColor;
}

.spaceName {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,16 @@
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/
/* this represent a triple state checkbox */

/*!
#set ($iconNames = ['check', 'cross'])
#set ($icons = {})
#foreach ($iconName in $iconNames)
#set ($discard = $icons.put($iconName, $services.icon.renderHTML($iconName)))
#end
#[[*/
// Start JavaScript-only code.
(function(icons) {
"use strict";
define('users-and-groups-translation-keys', {
prefix: 'platform.core.rightsManagement.',
keys: [
Expand Down Expand Up @@ -57,44 +66,44 @@ window.MSCheckbox = Class.create({
this.state = defaultState;
this.states = [0,1,2]; // 0 = undefined; 1 = allow, 2 = deny
this.nrstates = this.states.length;
this.images = [
"$xwiki.getSkinFile('js/xwiki/usersandgroups/img/none.png')",
"$xwiki.getSkinFile('js/xwiki/usersandgroups/img/allow.png')",
"$xwiki.getSkinFile('js/xwiki/usersandgroups/img/deny1.png')"
this.stateClasses = [
"none",
"yes",
"no"
];



this.button = document.createElement("button");
this.button.className = "rights-edit";
this.button.className = "btn btn-default btn-xs rights-edit";
this.button.addEventListener('click', this.createClickHandler(this));

var img = document.createElement("img");

this.button.appendChild(img);

$(domNode).appendChild(this.button);
this.draw(this.state);
this.draw();
},

/**
* @todo Draw with the current this.state, don't pass as an argument.
*/
draw: function(state)

draw: function()
{
//Change display image
var img = this.button.firstChild;
img.src = this.images[state];
//Change the display icon
if(this.state === 0) {
this.button.innerHTML = '';
}
if(this.state === 1) {
this.button.innerHTML = icons.check;
}
if(this.state === 2) {
this.button.innerHTML = icons.cross;
}

this.button.classList.add(this.stateClasses[this.state]);

//Update the description of the button for accessibility.
var button = this.button;
var state = this.state;
require(['xwiki-l10n!users-and-groups-translation-keys'], function(l10n) {
var alts = [
l10n['undefined'],
l10n['allowed'],
l10n['denied']
];
img.alt = alts[state];
button.title = alts[state];
});
},
Expand All @@ -105,13 +114,16 @@ window.MSCheckbox = Class.create({

next: function()
{
// Reinitialize class list
this.button.classList.remove(this.stateClasses[this.state]);

this.state = this.nextState();
if (this.table != undefined) {
// TODO: Just update the cache, don't invalidate the row, once the rights are as stored as an
// array, and not as a string.
delete this.table.fetchedRows[this.idx];
}
this.draw(this.state);
this.draw();
},

/* Confirmation cases:
Expand Down Expand Up @@ -230,6 +242,7 @@ window.MSCheckbox = Class.create({
}
}
});
}).apply(']]#', $jsontool.serialize([$icons]));

/**
* user list element creator. Used in adminusers.vm.
Expand Down Expand Up @@ -542,22 +555,20 @@ function setBooleanPropertyFromLiveCheckbox(self, saveDocumentURL, configuration
var saveURL = "$xwiki.getURL('XWiki.XWikiPreferences', 'save')";
var config = "XWiki.XWikiPreferences";
var objNum = "0";
if (saveDocumentURL != undefined && saveDocumentURL.length > 0) {
if (saveDocumentURL !== undefined && saveDocumentURL.length > 0) {
saveURL = saveDocumentURL;
}
if (configurationClassName != undefined && configurationClassName.length > 0) {
if (configurationClassName !== undefined && configurationClassName.length > 0) {
config = configurationClassName;
}
if (objectNumber != undefined) {
if (objectNumber !== undefined) {
objNum = objectNumber;
}
var pivot = self;
var newAlt = "yes";
var newSrc = "$xwiki.getSkinFile('js/xwiki/usersandgroups/img/allow-black.png')";
var newChecked = "checked";
var setValue = "1";
if (self.getAttribute('alt') == "yes") {
newAlt = "no";
newSrc = "$xwiki.getSkinFile('js/xwiki/usersandgroups/img/none.png')";
if (self.getAttribute('checked') === "checked") {
newChecked = "";
setValue = "0";
}
var paramMap = {};
Expand All @@ -566,8 +577,7 @@ function setBooleanPropertyFromLiveCheckbox(self, saveDocumentURL, configuration
paramMap["parameters"]["ajax"] = "1";
paramMap["parameters"]["comment"] = "$services.localization.render('authenticate_viewedit_savecomment')";
paramMap["onSuccess"] = function() {
pivot.alt = newAlt;
pivot.src = newSrc;
pivot.setAttribute('checked',newChecked);
}
new Ajax.Request(saveURL, paramMap);
};
Expand All @@ -589,15 +599,13 @@ function setGuestExtendedRights(self)
parameters: {"XWiki.XWikiPreferences_0_authenticate_view" : "0"},
onSuccess: function() {
pivot.alt = "no";
pivot.src = "$xwiki.getSkinFile('js/xwiki/usersandgroups/img/none.png')";
}});
} else {
new Ajax.Request(url, {
method: 'post',
parameters: {"XWiki.XWikiPreferences_0_authenticate_edit" : "0"},
onSuccess: function() {
pivot.alt = "no";
pivot.src = "$xwiki.getSkinFile('js/xwiki/usersandgroups/img/none.png')";
}});
}
} else {
Expand All @@ -607,15 +615,13 @@ function setGuestExtendedRights(self)
parameters: {"XWiki.XWikiPreferences_0_authenticate_view" : "1"},
onSuccess: function() {
pivot.alt = "yes";
pivot.src = "$xwiki.getSkinFile('js/xwiki/usersandgroups/img/allow-black.png')";
}});
} else {
new Ajax.Request(url, {
method: 'post',
parameters: {"XWiki.XWikiPreferences_0_authenticate_edit" : "1"},
onSuccess: function() {
pivot.alt = "yes";
pivot.src = "$xwiki.getSkinFile('js/xwiki/usersandgroups/img/allow-black.png')";
}});
}
}
Expand Down
Loading