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

V1.0 j cmodif #49

Closed
wants to merge 2 commits into from
Closed

V1.0 j cmodif #49

wants to merge 2 commits into from

Conversation

jcolomb
Copy link
Contributor

@jcolomb jcolomb commented Jun 18, 2019

chnage icons, test how it looks like

@jcolomb
Copy link
Contributor Author

jcolomb commented Jun 18, 2019

@dannycolin any way to preview the change before merging?

@jcolomb
Copy link
Contributor Author

jcolomb commented Jun 18, 2019

link to issue #48

@jcolomb jcolomb requested a review from dannycolin June 18, 2019 12:18
@dannycolin
Copy link
Member

dannycolin commented Jun 18, 2019

Oh sorry. I misunderstood what you asked last time. The SVG icons have a ARIA role=presentation and therefore they aren't visible for screen reader users. I think it'd be best to add something like the following code just before the <ul> tag.

<p>
  <span class="show-for-sr">Status:</span>
  <span class="label">{{ module.status }}</span>
</p>

{{ module.status }} can be set to planned, in development, online in the metadata of each module.

Sidenote related to your issue:

I use SVG instead of PNG because it doesn't pixel-lze whatever the resolution is and I save 14 http requests so the page loading is faster ;).

@jcolomb
Copy link
Contributor Author

jcolomb commented Jun 19, 2019

http://osmooc.dannycolin.com/ there we see the icons, what do you mean ?

by the way, the website still show the modules in the wrong order, changes not pushed or not deployed?

@jcolomb
Copy link
Contributor Author

jcolomb commented Jun 19, 2019

It is a design issue, expanding the code, we could change the icon more (black and white for modules which are not online for instance), I think it will make clear on the first glimpse what is available and what is planned. I just use simple writing for testing and then we could ask if people have some more design ideas...

@dannycolin
Copy link
Member

dannycolin commented Jun 19, 2019

by the way, the website still show the modules in the wrong order, changes not pushed or not deployed?

I know. I didn't had the time to implement it yet :(.

It is a design issue, expanding the code, we could change the icon more (black and white for modules which are not online for instance), I think it will make clear on the first glimpse what is available and what is planned. I just use simple writing for testing and then we could ask if people have some more design ideas...

It is possible to add something like gray scaled icons but still we shouldn't rely solely on text inside icons or colors because it isn't accessible.

For example, what if someone is color blind. They won't be able to see the status of a module based on the color of the icons. Also, it is possible to make an SVG text accessible (vs. a png) but it demands a bit more work. You need to do it manually because software like Illustrator or Inkscape don't generate 100% clean svg code all the time (there's doctype in the file you uploaded as an example). Plus, you also need to test it with a screen reader to be sure and that adds more work.

IOW, I'm trying to keep it simple right now so we can launch the redesign as soon as possible and there's also the possibility that we migrate to Hugo because it has a builtin support for translation. However, I'm still open to any PR that is WCAG 2.1 compliant tho.

I hope it answer your concerns about the design upgradability.

@jcolomb
Copy link
Contributor Author

jcolomb commented Jun 19, 2019

I do not need to access the svg text, the status and link to the original icon comes from the metadata file, a R code produce the new icons form the orginal one. The svg is probably not that clean, since I had to go via saving a png first. But I have no time to look into that and cannot test how it looks like once deployed. In most website I saw, people use png files for icons, it seems we have different views on what is "simple" :=)

You have lost me with "WCAG 2.1"

what is a screen reader?

This icon transformation will work with hugo.

@jcolomb
Copy link
Contributor Author

jcolomb commented Jun 19, 2019

BTW, trying to test deployment by forking and changing the master, but no success. Is there any tricks?

maybe a css issue:

Your SCSS file assets/css/app.scss has an error on line 277: Invalid CSS after "&.": expected class name, was "#fff". For more information, see https://help.github.com/en/articles/page-build-failed-invalid-sass-or-scss.

@dannycolin
Copy link
Member

dannycolin commented Jun 19, 2019

The svg is probably not that clean, since I had to go via saving a png first.

Isn't it possible to directly with the svg file instead of a PNG in R? Also, you can use svgo to clean up your svg version if needed.

In most website I saw, people use png files for icons, it seems we have different views on what is "simple" :=)

Sadly, lots of web developers don't care or know enough about web accessibility and hence PNG/JPG (raster graphics) are more popular :(. Oh and by simple, I just mean less work. ;)

You have lost me with "WCAG 2.1"
what is a screen reader?

It is the web standard that describes what's web accessibility and how it should be implemented. See: https://www.w3.org/TR/WCAG21/ There's also an article on MDN that is a bit easier to read than the w3 standard.

A screen reader is a software that reads digital text aloud and it is use by people with visual impairments (blindness, low-level vision, and color blindness).

This icon transformation will work with hugo.

Yes but there's a few differences between Hugo and Jekyll for the code to include a file in another one and how the code of a template interact with the metadata of a page. So, that part of the code will need to be ported (rewritten) and it means that the more functionalities we had to the Jekyll website the more we'll have to rewrite if we migrate.

@dannycolin
Copy link
Member

BTW, trying to test deployment by forking and changing the master, but no success. Is there any tricks?

You mean the v1.0 branch?

maybe a css issue:

Your SCSS file assets/css/app.scss has an error on line 277: Invalid CSS after "&.": expected class name, was "#fff". For more information, see https://help.github.com/en/articles/page-build-failed-invalid-sass-or-scss.

I can't reproduce the error on my side. The v1.0 branch use a slighty different build process to generate the code. The documentation is still in a "work in progress" state. It'll be ready for the launch tho. Inside the project folder, you need to do bundle install to make sure you have all the dependencies for SCSS. Also, there's no need to push it to github. You should be able to preview your change locally with the command rake serve:dev.

@dannycolin dannycolin closed this Jul 8, 2019
@jcolomb jcolomb deleted the v1.0_JCmodif branch November 29, 2019 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants