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-21492: Underline inline links #2694

Merged
merged 37 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
14d93af
XWIKI-21492: Underline inline links
Sereza7 Oct 24, 2023
d97e093
XWIKI-21492: Underline inline links
Sereza7 Oct 30, 2023
59e3c22
XWIKI-21492: Underline inline links
Sereza7 Oct 30, 2023
78dc29c
XWIKI-21492: Underline inline links
Sereza7 Nov 2, 2023
4959418
XWIKI-21492: Underline inline links
Sereza7 Oct 24, 2023
ea1e0e4
XWIKI-21492: Underline inline links
Sereza7 Oct 30, 2023
5e6335b
XWIKI-21492: Underline inline links
Sereza7 Oct 30, 2023
2ec89a8
XWIKI-21492: Underline inline links
Sereza7 Nov 2, 2023
b841c0d
Merge remote-tracking branch 'origin/XWIKI-21492' into XWIKI-21492
Sereza7 Nov 22, 2023
3855a6a
XWIKI-21492: Underline inline links
Sereza7 Dec 5, 2023
864b32d
XWIKI-21492: Underline inline links
Sereza7 Dec 6, 2023
51295dc
Merge branch 'xwiki:master' into XWIKI-21492
Sereza7 Dec 6, 2023
43b7ce3
XWIKI-21492: Underline inline links
Sereza7 Dec 6, 2023
766f617
Merge branch 'xwiki:master' into XWIKI-21492
Sereza7 Dec 18, 2023
344d906
XWIKI-21492: Underline inline links
Sereza7 Dec 18, 2023
71191ee
XWIKI-21492: Underline inline links
Sereza7 Dec 18, 2023
f689000
XWIKI-21492: Underline inline links
Sereza7 Dec 18, 2023
af84d5b
Merge branch 'xwiki:master' into XWIKI-21492
Sereza7 Jan 3, 2024
acb4768
XWIKI-21492: Underline inline links
Sereza7 Jan 3, 2024
18c1f4d
XWIKI-21492: Underline inline links
Sereza7 Jan 3, 2024
b4e00a6
XWIKI-21492: Underline inline links
Sereza7 Jan 3, 2024
6b9d08c
XWIKI-21492: Underline inline links
Sereza7 Jan 5, 2024
9b88f8f
XWIKI-21492: Underline inline links
Sereza7 Jan 5, 2024
7e5d8f0
XWIKI-21492: Underline inline links
Sereza7 Jan 5, 2024
5da3f61
Merge branch 'xwiki:master' into XWIKI-21492
Sereza7 Jan 30, 2024
d2941bf
XWIKI-21492: Underline inline links
Sereza7 Jan 30, 2024
32a7cca
XWIKI-21492: Underline inline links
Sereza7 Jan 31, 2024
7bf473e
XWIKI-21492: Underline inline links
Sereza7 Feb 6, 2024
eba9834
Revert "XWIKI-21492: Underline inline links"
Sereza7 Feb 9, 2024
3ae794d
XWIKI-21492: Underline inline links
Sereza7 Feb 12, 2024
09af3b4
XWIKI-21492: Underline inline links
Sereza7 Mar 12, 2024
6d02af5
XWIKI-21492: Underline inline links
Sereza7 Mar 22, 2024
cc1aae8
Merge branch 'master' into XWIKI-21492
Sereza7 Apr 3, 2024
0a6a53f
Merge branch 'xwiki:master' into XWIKI-21492
Sereza7 Apr 17, 2024
674b74f
XWIKI-21492: Underline inline links
Sereza7 Apr 17, 2024
5c627bc
XWIKI-21492: Underline inline links
Sereza7 Apr 17, 2024
df185ba
XWIKI-21492: Underline inline links
Sereza7 Apr 17, 2024
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 @@ -101,7 +101,8 @@ protected void createClass(BaseClass xclass)
xclass.addTextField("imaccount", "imaccount", 30);
xclass.addStaticListField("editor", "Default Editor", "Text|Wysiwyg");
xclass.addStaticListField("usertype", "User type", "Simple|Advanced", "Simple");
xclass.addBooleanField("accessibility", "Enable extra accessibility features", "yesno");
xclass.addStaticListField("font_size", "Font size", "Regular|Large|Larger|Largest", "Regular");
xclass.addStaticListField("underline", "Underline links", "Yes|Only inline links|No", "Only inline links");
Sereza7 marked this conversation as resolved.
Show resolved Hide resolved
xclass.addBooleanField("displayHiddenDocuments", "Display Hidden Documents", "yesno");
xclass.addTimezoneField(TIMEZONE_FIELD, "Time Zone", 30);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -702,10 +702,11 @@ platform.core.profile.phone=Phone
platform.core.profile.address=Address
platform.core.profile.editor=Default editor to use
platform.core.profile.userType=User Type
platform.core.profile.enableAccessibility=Enable extra accessibility features
platform.core.profile.displayHiddenDocuments=Display hidden pages
platform.core.profile.timezone=Timezone
platform.core.profile.extensionConflictSetup=Enable extension conflict setup
platform.core.profile.accessibility.underline=Underlining
Sereza7 marked this conversation as resolved.
Show resolved Hide resolved
platform.core.profile.accessibility.fontSize=Font size

platform.core.profile.category.settings=Settings
platform.core.profile.category.profile=Profile
Expand All @@ -728,6 +729,7 @@ platform.core.profile.section.sendMessage=Send Message
platform.core.profile.section.activity=My Activity Stream
platform.core.profile.section.activityof=Activity stream of {0}
platform.core.profile.section.displayPreferences=Display Preferences
platform.core.profile.section.accessibilityPreferences=Accessibility preferences
platform.core.profile.section.localizationPreferences=Localization Preferences
platform.core.profile.section.editorPreferences=Editor Preferences
platform.core.profile.section.extensionPreferences=Extensions Preferences
Expand Down Expand Up @@ -5649,6 +5651,11 @@ core.viewers.diff.previousVersion=Previous version
core.viewers.diff.nextChange=Next change
core.viewers.diff.previousChange=Previous change

#######################################
## until 15.10.1
#######################################
platform.core.profile.enableAccessibility=Enable extra accessibility features

## Used to indicate where deprecated keys end
#@deprecatedend

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@
<syntaxId>plain/1.0</syntaxId>
<hidden>true</hidden>
<content>XWiki.XWikiUsers_displayHiddenDocuments.hint=The wiki contains documents that are not displayed by default. These hidden documents represent technical content, like application classes, configuration pages, macros, styles, scripts, etc.
XWiki.XWikiUsers_accessibility.hint=Extra accessibility will enable various visual enhancements like: bigger fonts, underlined links, etc.
Sereza7 marked this conversation as resolved.
Show resolved Hide resolved
XWiki.XWikiUsers_font_size.hint=Change the font size for the main text (in px).
Copy link
Contributor

Choose a reason for hiding this comment

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

From my understanding, there are only a few possible values (Regular, Large, Larger, Largest) and no pixel values, so I don't understand the "in px" comment. Also, I think this is missing translations for the values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 344d906 and acb4768 👍

XWiki.XWikiUsers_underline.hint=Set your underline preferences. Default is to underline only inline links.
Sereza7 marked this conversation as resolved.
Show resolved Hide resolved
XWiki.XWikiUsers_timezone.hint=Use a specific timezone so that the dates reflect your current location.
XWiki.XWikiUsers_editor.hint=Choose what editor will be the default one, overriding the default editor set globally. If not defined, a default editor is chosen, depending on what is being edited.
XWiki.XWikiUsers_usertype.hint=Choose what type of user will be the default one. Advanced users will have access to multiple editing features, terminal page creation, etc.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,169 @@
<syntaxId>xwiki/2.0</syntaxId>
<hidden>true</hidden>
<content/>
<object>
<name>XWiki.XWikiUserPreferencesSheet</name>
<number>0</number>
<className>XWiki.StyleSheetExtension</className>
<guid>4d653eae-a25f-40a2-abe9-719e06a61185</guid>
<class>
<name>XWiki.StyleSheetExtension</name>
<customClass/>
<customMapping/>
<defaultViewSheet/>
<defaultEditSheet/>
<defaultWeb/>
<nameField/>
<validationScript/>
<cache>
<cache>0</cache>
<defaultValue>long</defaultValue>
<disabled>0</disabled>
<displayType>select</displayType>
<freeText>forbidden</freeText>
<largeStorage>0</largeStorage>
<multiSelect>0</multiSelect>
<name>cache</name>
<number>5</number>
<prettyName>Caching policy</prettyName>
<relationalStorage>0</relationalStorage>
<separator> </separator>
<separators>|, </separators>
<size>1</size>
<unmodifiable>0</unmodifiable>
<values>long|short|default|forbid</values>
<classType>com.xpn.xwiki.objects.classes.StaticListClass</classType>
</cache>
<code>
<contenttype>PureText</contenttype>
<disabled>0</disabled>
<editor>PureText</editor>
<name>code</name>
<number>2</number>
<prettyName>Code</prettyName>
<restricted>0</restricted>
<rows>20</rows>
<size>50</size>
<unmodifiable>0</unmodifiable>
<classType>com.xpn.xwiki.objects.classes.TextAreaClass</classType>
</code>
<contentType>
<cache>0</cache>
<disabled>0</disabled>
<displayType>select</displayType>
<freeText>forbidden</freeText>
<largeStorage>0</largeStorage>
<multiSelect>0</multiSelect>
<name>contentType</name>
<number>6</number>
<prettyName>Content Type</prettyName>
<relationalStorage>0</relationalStorage>
<separator> </separator>
<separators>|, </separators>
<size>1</size>
<unmodifiable>0</unmodifiable>
<values>CSS|LESS</values>
<classType>com.xpn.xwiki.objects.classes.StaticListClass</classType>
</contentType>
<name>
<disabled>0</disabled>
<name>name</name>
<number>1</number>
<prettyName>Name</prettyName>
<size>30</size>
<unmodifiable>0</unmodifiable>
<classType>com.xpn.xwiki.objects.classes.StringClass</classType>
</name>
<parse>
<disabled>0</disabled>
<displayFormType>select</displayFormType>
<displayType>yesno</displayType>
<name>parse</name>
<number>4</number>
<prettyName>Parse content</prettyName>
<unmodifiable>0</unmodifiable>
<classType>com.xpn.xwiki.objects.classes.BooleanClass</classType>
</parse>
<use>
<cache>0</cache>
<disabled>0</disabled>
<displayType>select</displayType>
<freeText>forbidden</freeText>
<largeStorage>0</largeStorage>
<multiSelect>0</multiSelect>
<name>use</name>
<number>3</number>
<prettyName>Use this extension</prettyName>
<relationalStorage>0</relationalStorage>
<separator> </separator>
<separators>|, </separators>
<size>1</size>
<unmodifiable>0</unmodifiable>
<values>currentPage|onDemand|always</values>
<classType>com.xpn.xwiki.objects.classes.StaticListClass</classType>
</use>
</class>
<property>
<cache>default</cache>
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure this is working and also, e.g., updated when user preferences are saved? I think a more robust solution could be one of the following:

  1. Put the user preference into a URL parameter that is passed when the SSX is loaded.
  2. Transform the user preference into a data attribute or class on a central element (like body or even HTML) and use CSS selectors based on that. This has the advantage that other CSS can easily adapt its styles when it makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The line at #2694 (comment) was in charge of reloading the SSX no matter the cache. I updated the cache policy to account for removing this line, disabling cache for this file.
Ideally we should only invalidate cache for this skin extension when the user preference change, but technically it's not that easy.
Addressed in 6b9d08c 👍
See https://up1.xwikisas.com/#i_R7F8zyuo9LADkeBg-vgw for a demo. In this demo, I just changed the font size since it's the easiest to spot out quickly, but it works the same for the link underlining.

</property>
<property>
<code>## Add extra style based on the preferences of the user
## Underlining of links
#set ($underlining = "$!{xwiki.getUserPreference('underline')}")
#if ($underlining == 'Yes')
a {
text-decoration: underline;
}
#end
#if ($underlining == 'Only inline links')
p, span, div {
&amp; &gt; .wikilink,
&amp; &gt; .wikiexternallink,
&amp; &gt; .wikiinternallink,
&amp; &gt; .wikicreatelink {
&amp; &gt; a {
text-decoration: underline;
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems strange, why are links in list items, definition lists etc. not underlined?

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 reused the selector we converged towards at the end of this previous PR. It can definitely still be improved, I'll try to make it a bit better, but from what I can remember there's no perfect solution because we can't 'see' text nodes in CSS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't checked what kind of elements we have normally as parents of links, but in general this approach seems very fragile and error-prone to me. I would have preferred an approach where we underline links with a broad, generic selector (to, e.g., generically match content and panels) and then use specific selectors to remove the underline where we don't want it. Just to avoid that we have cases where users don't understand why one link in the content is underlined and another isn't.

This is my opinion, I don't know how others see this and if they would prefer either that potentially some links in the content aren't underlined or that potentially too many links are underlined, but that underlining is mostly consistent.

Copy link
Contributor Author

@Sereza7 Sereza7 Jan 31, 2024

Choose a reason for hiding this comment

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

I updated the selector in d2941bf .
I've split it in different sections with easy objectives:

  1. Ruleset for all links in content.
  2. Exceptions to add underlining outside of content
  3. Exceptions to remove underlining inside content
  4. Helper classes to "force" the decoration on an anchor. Note that this class only makes a difference with the underline inline link preference chosen. With all links underlined or no link underlined, the user preference is predominant.

The advantage of this is that it's more consistent and easier to understand than the previous one. The disadvantage is that it's not really consistent with the inline link definition. We might need to update this selector soon to address leftover accessibility violations. There's some of those violations everywhere in the build right now so I can't test them all out...

}
}
}
#end
#if ($underlining == 'No')
a {
text-decoration: none;
}
#end

#set ($fontSizePreference = $!{xwiki.getUserPreference('font_size')})
## Default font size is 14px, which is equivalent to 140%
#if ($fontSizePreference == 'Large')
.skin-flamingo {
Sereza7 marked this conversation as resolved.
Show resolved Hide resolved
font-size: 160%;
}
#end
#if ($fontSizePreference == 'Larger')
.skin-flamingo {
font-size: 180%;
}
#end
#if ($fontSizePreference == 'Largest')
.skin-flamingo {
font-size: 200%;
}
#end</code>
</property>
<property>
<contentType>CSS</contentType>
</property>
<property>
<name></name>
</property>
<property>
<parse>1</parse>
</property>
<property>
<use>always</use>
</property>
</object>
<object>
<name>XWiki.XWikiUserPreferencesSheet</name>
<number>0</number>
Expand Down Expand Up @@ -287,7 +450,19 @@
&lt;h1&gt;$escapetool.xml($services.localization.render('platform.core.profile.section.displayPreferences'))&lt;/h1&gt;
&lt;dl&gt;
#displayField('displayHiddenDocuments', 'platform.core.profile.displayHiddenDocuments', '0')
#displayField('accessibility' , 'platform.core.profile.enableAccessibility' , '0')
&lt;/dl&gt;
&lt;h1&gt;$escapetool.xml($services.localization.render('platform.core.profile.section.accessibilityPreferences'))&lt;/h1&gt;
&lt;dl&gt;
#set ($defaultValue = $xwiki.getXWikiPreference('font_size'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Where should that value come from?

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 don't understand this comment.
Here, I followed the same pattern as other properties.
From what I understand, the value comes from the edited user preferences.

Copy link
Contributor

Choose a reason for hiding this comment

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

From what I understand, the value comes from the edited user preferences.

No, this loads a value from the wiki configuration, i.e., it reads values from the XWiki.XWikiPreferences document - which has an accessibility field, but no font_size or underline. Actually, I think $!{xwiki.getUserPreference('accessibility')} reads this accessibility field from the wiki (or main wiki) configuration, too, when it is not set in the user profile/subwiki.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In regards to that, I added the values in the Xwiki and space preferences in 32a7cca .
👍

#if ("$!defaultValue" == '')
#set ($defaultValue = 'Regular')
#end
#displayField('font_size' , 'platform.core.profile.accessibility.fontSize' , $defaultValue)
Sereza7 marked this conversation as resolved.
Show resolved Hide resolved
#set ($defaultValue = $xwiki.getXWikiPreference('underline'))
#if ("$!defaultValue" == '')
#set ($defaultValue = 'Only inline links')
#end
#displayField('underline' , 'platform.core.profile.accessibility.underline' , $defaultValue)
Sereza7 marked this conversation as resolved.
Show resolved Hide resolved
&lt;/dl&gt;
&lt;h1&gt;$escapetool.xml($services.localization.render('platform.core.profile.section.localizationPreferences'))&lt;/h1&gt;
&lt;dl&gt;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,8 @@ $originalUrl?$queryString## prevent line break in output

<link href="#concatenateQueryParameter($defaultStyleURL, $urlParameters)" rel="stylesheet" type="text/css" media="all" />
<link href="#concatenateQueryParameter($escapetool.xml($xwiki.getSkinFile('print.css', true)), $urlParameters)" rel="stylesheet" type="text/css" media="#if ($printss)all#{else}print#{end}" />
#set ($a11y = "$!{request.getCookie('a11y').getValue()}")
#if ($a11y == '')
#set ($a11y = "$!{xwiki.getUserPreference('accessibility')}")
#end
#if ($a11y == '1')
<link href="$escapetool.xml($xwiki.getSkinFile('css/accessibility.css', true))" rel="stylesheet" type="text/css" media="all" />
#end
## User preferences stylesheet
#set($discard = $xwiki.ssx.use('XWiki.XWikiUserPreferencesSheet'))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not necessary when the SSX has "always" as "use".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 344d906 👍

## The stylesheets configuration option allows to override the alternate stylesheets
## style1.css, style2.css and style3.css
#set ($stylesheets = $xwiki.getSpacePreference('stylesheets'))
Expand Down

This file was deleted.