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-22495: Missing text content in the resource picker dropdown #3612

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
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 @@ -19,6 +19,23 @@
*/
#template('colorThemeInit.vm')

/* CKEditor contains a CSS reset. It works with its own style sheets and does not use the ones in XWiki.
However, we want `.sr-only` from XWiki to still be usable in our CKEditor plugins.
We need to redefine the XWiki styles of this class to have better priority than the CKEditor CSS reset.
Without this, the elements with this class are still shown
which would be different from the behaviour of `.sr-only` anywhere else in XWiki (as described in our doc).
This redefinition allows for a more consistent behaviour of the `.sr-only` class. */
.resourcePicker .sr-only {
position: absolute;
width: 1px;
height: 1px;
padding: 0;
margin: -1px;
overflow: hidden;
clip: rect(0, 0, 0, 0);
border: 0;
}
Copy link
Member

Choose a reason for hiding this comment

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

If any other sr-only elements are needed in our CKEditor UI, we might want to write a style rule for all of them with .cke_reset_all .sr-only.

I think I prefer the generic rule because there's a high chance we'll need this on other parts of CKEditor UI. Maybe put this in CKEditor.EditSheet .


.resourcePicker .dropdown-menu > .active > a,
.resourcePicker .dropdown-menu > .active > a:hover,
.resourcePicker .dropdown-menu > .active > a:focus {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
*/
define('resourcePickerTranslationKeys', [], [
'attach.hint',
'doc.hint'
'doc.hint',
'dropdown.toggle.title'
]);

define('resourcePicker', [
Expand All @@ -35,7 +36,8 @@ define('resourcePicker', [
'<button type="button" class="resourceType btn btn-default">' +
'<span class="icon">' +
'</button>' +
'<button type="button" class="btn btn-default dropdown-toggle" data-toggle="dropdown">' +
'<button type="button" class="btn btn-default dropdown-toggle" data-toggle="dropdown">' +
'<span class="sr-only"></span>' +
Comment on lines +39 to +40
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to keep the indentation.

'<span class="caret"></span>' +
'</button>' +
'<ul class="resourceTypes dropdown-menu dropdown-menu-right"></ul>' +
Expand All @@ -48,6 +50,8 @@ define('resourcePicker', [
options = options || {};

var resourcePicker = $(resourcePickerTemplate);
resourcePicker.find('button.dropdown-toggle').first().attr('title', translations.get('dropdown.toggle.title'));
resourcePicker.find('button.dropdown-toggle .sr-only').first().text(translations.get('dropdown.toggle.title'));
Comment on lines +53 to +54
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
resourcePicker.find('button.dropdown-toggle').first().attr('title', translations.get('dropdown.toggle.title'));
resourcePicker.find('button.dropdown-toggle .sr-only').first().text(translations.get('dropdown.toggle.title'));
resourcePicker.find('button.dropdown-toggle').attr('title', translations.get('dropdown.toggle.title'));
resourcePicker.find('button.dropdown-toggle .sr-only').text(translations.get('dropdown.toggle.title'));

No need to limit the collection to the first item.

resourcePicker.data("options", options);
element.on('selectResource', onSelectResource).hide().after(resourcePicker);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ resource.user.placeholder=alias

resourcePicker.attach.hint=Select an attachment
resourcePicker.doc.hint=Select a page
resourcePicker.dropdown.toggle.title=Toggle the display of resource types.

entityResourceSuggester.doc.placeholder=Find a page...
entityResourceSuggester.attach.placeholder=Find an attachment...
Expand Down