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.

Sign up for GitHub

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-22339: Icon picker is not keyboard operable #3800

Merged
merged 3 commits into from
Jan 16, 2025
Merged

XWIKI-22339: Icon picker is not keyboard operable #3800

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
113759b
XWIKI-22339: Icon picker is not keyboard operable
Sereza7 Jan 14, 2025
9504c39
XWIKI-22339: Icon picker is not keyboard operable
Sereza7 Jan 14, 2025
0f948a2
XWIKI-22339: Icon picker is not keyboard operable
Sereza7 Jan 14, 2025
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
49 changes: 42 additions & 7 deletions ...iki-platform-icon/xwiki-platform-icon-ui/src/main/resources/IconThemesCode/IconPicker.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,21 @@ require(['jquery', 'xwiki-icon-picker'], function($) {
for (var i=0; i < iconTheme.length; ++i) {
// Display the icon
if (selectedIconName === '' || iconTheme[i].name.includes(selectedIconName)) {
var displayer = $(document.createElement('div'));
var displayer = $(document.createElement('button'));
Copy link
Member

Choose a reason for hiding this comment

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

any reason to not use $('<button>') (see https://api.jquery.com/jQuery/#jQuery2)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really, just keeping the changes minimal. AFAICS all the elements created in this file use this pattern. I figured keeping this pattern is the safest choice (though I doubt it would change anything in any situation).

Should I replace all the instantiations with the pattern you proposed?

No changes applied yet.

Copy link
Member

Choose a reason for hiding this comment

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

Should I replace all the instantiations with the pattern you proposed?

Probably not, leave it like this.

displayer.addClass('btn');
iconListSection.append(displayer);
displayer.addClass('xwikiIconPickerIcon');
var imageDiv = $(document.createElement('div'));
var imageDiv = $(document.createElement('p'));
imageDiv.addClass('xwikiIconPickerIconImage').html(iconTheme[i].render);
displayer.append(imageDiv);
var iconNameSpan = $(document.createElement('span'));
iconNameSpan.addClass('xwikiIconPickerIconName').text(iconTheme[i].name);
var iconNameSpan = $(document.createElement('p'));
// Usually, the icon name is just one "word" in snakecase. e.g. arrow_refresh_small
// We add Line Break Opportunity elements in the middle of this word so that it's cut into nice to read
// substrings.
let iconNameWithLineBreakOpportunities = iconTheme[i].name
.replaceAll("-","&lt;wbr/&gt;-")
.replaceAll("_","&lt;wbr/&gt;_");
iconNameSpan.addClass('xwikiIconPickerIconName').html(iconNameWithLineBreakOpportunities);
displayer.append(iconNameSpan);
// Change the input value when the icon is clicked
displayer.on('click', function(event) {
Expand Down Expand Up @@ -373,7 +380,7 @@ require(['jquery', 'xwiki-icon-picker'], function($) {
}

/**
* Create the piccker
* Create the picker
*/
var createPicker = function (input) {
// If the picker already exists
Expand All @@ -392,7 +399,7 @@ require(['jquery', 'xwiki-icon-picker'], function($) {
xwikiCurrentIconsPicker = $(document.createElement('div'));
xwikiCurrentIconsPicker.addClass('xwikiIconPickerContainer');
// Insert the picker in the DOM
$(body).append(xwikiCurrentIconsPicker);
xwikiCurrentIconsPicker.insertAfter(currentInput);
// Set the position
setPickerPosition();
// Add the icon list section
Expand Down Expand Up @@ -447,6 +454,13 @@ require(['jquery', 'xwiki-icon-picker'], function($) {
lastTimeout = setTimeout(reloadIconList, 500);
}
});
iconThemeSelector.on('keyup', function(event) {
// Close the picker when the user press 'escape'.
// See: http://stackoverflow.com/questions/1160008/which-keycode-for-escape-key-with-jquery
if (event.which == 27) {
closePicker();
}
});
}

/**
Expand Down Expand Up @@ -613,10 +627,17 @@ require(['jquery', 'xwiki-icon-picker'], function($) {
.xwikiIconPickerList {
overflow-y: scroll;
height: 240px;
display: flex;
flex-wrap: wrap;
gap: .3em;
}

/* Avoid stretching on the Loader element. */
.xwikiIconPickerList &gt; img {
object-fit: contain;
}

.xwikiIconPickerIcon {
float: left;
height: 80px;
width: 100px;
text-align: center;
Expand All @@ -625,6 +646,7 @@ require(['jquery', 'xwiki-icon-picker'], function($) {
overflow: hidden;
padding: 10px;
cursor: pointer;
background-color: transparent;
}

.xwikiIconPickerIcon:hover {
Expand All @@ -633,13 +655,26 @@ require(['jquery', 'xwiki-icon-picker'], function($) {

.xwikiIconPickerIconImage {
font-size: 24px;
line-height: 1;
margin: 0;
}

.xwikiIconPickerIconImage img{
width: 24px;
}

.xwikiIconPickerIcon .xwikiIconPickerIconName {
/* We want to limit the number of lines in the name so that it always fit inside the standard button size.
The value below is just about twice the line-height. */
max-height: 2.9em;
/* For the few names that need three lines, we make sure the user can scroll to read the whole name. */
overflow-y: scroll;
overflow-x: hidden;
/* Make sure the name wraps onto multiple lines instead of overflowing. */
white-space: pre-wrap;
word-break: break-word;
}

.xwikiIconPickerIconThemeSelector {
width: 100%;
background-color: $theme.menuBackgroundColor;
Expand Down
Loading