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

use Font Awesome fonts for icons #1319

Merged
merged 2 commits into from
May 4, 2022
Merged

use Font Awesome fonts for icons #1319

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f86b92
use Font Awesome fonts for icons, including copy to clipboard icon
osma May 3, 2022
4a780c2
add Font Awesome Solid set as well, needed by some widgets/plugins
osma May 4, 2022
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
12 changes: 3 additions & 9 deletions resource/css/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1970,22 +1970,16 @@ span.date-info {
.copy-clipboard {
color: var(--medium-color);
border-radius: 0;
vertical-align: unset;
vertical-align: inherit;
display: inline-block;
font-size: var(--font-size);
border: 0;
margin: 0;
padding: 0;
}

.prop-preflabel .copy-clipboard img {
vertical-align: text-bottom;
}

.copy-clipboard img {
width: 1em;
height: 1em;
vertical-align: text-top;
.prop-preflabel .copy-clipboard {
font-size: 20px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Only downside of fixed pixel size is for users that need to change page zoom,as the icon does not scale.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that applies nowadays. CSS pixels are not the same as screen pixels, and when you zoom in on a web page, browsers will happily adjust text sizes even if they are declared in px.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tested this using Firefox and Chromium. The icon scales along with the text in both browsers when you zoom in.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yup, when you zoom in in your browser that works. But vision impaired users normally have different font-sizes configured in their browsers.

In that case the web application using pixels is not always rendered correctly.

To test it with Chromium, for instance, go to Settings / Appearance. Then change Font Size that should be set to its default (24) to something like Huge 72 just to test.

image

image

In the GIF below, I'm showing SKOSMOS after changing the browser font size to Huge. Notice that the icon and the text are using the pixel size, ignoring that I've changed the font size (i.e. not relative like em or rem). Then I'm changing the font size to 1em in the icon, and then 1em for the CSS root variable.

GIFrecord_2022-05-04_232059

There are other cases where the fixed pixel size doesn't work or gets in the way of users that with different configurations, but I remember this one because I use a large font size in some devices — family history of poor vision, so I've already learned how to use a screen reader, and try to promote WCAG 2.1/Section 508 (US)/Web Accessibility Standard (NZ)/good practices/etc whenever I can 😬
I tried using em during the Bootstrap 5 work, but the final result was different than Bootstrap 3, so it was simpler to just copy the values from Bootstrap 3. I think for now we should use this 20px, and if we need to support accessibility standards like WCAG we can create a new issue later and see what's required (I think Europe adopted WCAG 2.1 too, but I only worked with a11y in NZ & Brazil).

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah right, thanks for the clarification and example. I first thought that you were commenting on the pixel value used in this PR just for a single icon, but this is a much more general issue which affects many elements in the Skosmos UI.

In fact we do have a requirement to follow WCAG level AA, or at least to document any deviations from it (there are many known a11y problems - and likely many unknown - in current Skosmos, unfortunately). There is an accessibility statement for Finto - in Finnish only - which contains a long list of known problems as well as aspects that have not been verified.

The best solution would probably be to avoid px sizes for fonts altogether. But implementing that should be planned in an issue and implemented as a PR (or several).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed! 👍 😀 Thanks @osma

}

.copy-clipboard:active, .copy-clipboard:focus, .copy-clipboard:active:focus {
Expand Down
Loading