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

Hex page updates #628

Open
fforres opened this issue Aug 28, 2018 · 6 comments
Open

Hex page updates #628

fforres opened this issue Aug 28, 2018 · 6 comments

Comments

@fforres
Copy link
Member

fforres commented Aug 28, 2018

For discussing #627 changes

@ogsilvaa
Copy link

The change in hexdex page is based in chapter page, so if navigator do not have JS enabled, then, chapter and hexdex do not work.

On the other hand, the description and height should be corrected.

'------

El cambio en la página hexdex, esta basado en la página chapter, por lo que si el navegador no tiene habilitado JS, entonces, chapter y hexdex no funcionarán.

Por otra parte, la descripción y la altura deben ser corregidos.

@fforres
Copy link
Member Author

fforres commented Aug 28, 2018

@ogsilvaa first of all, it is really cool that you decided to work on the hexdex pages! Thanks a lot for the contribution 🙏 !!

Over the pr, my 2 cents would be.

  • I like a lot the idea of having some visual description for the hexagons! 🤘
    Seeing them there, while looking really cool, does not show context or information on the chapters

    I think adding the description bellow each image and linking it to the image as an aria-describedby could be really cool 😄

  • The borders for some images have different height :/
    screen shot 2018-08-27 at 9 04 12 pm

  • For some images (Like Istanbul or Haarlem) the yellow background for the box is really similar to the logo background color.

    screen shot 2018-08-27 at 9 22 45 pm screen shot 2018-08-27 at 9 22 36 pm
  • Separating the logos per region/continent is such a coool idea!! 🎉 !! I really dig it!

Anyways, I'm in no way a gatekeeper on the site, just my 2 cents :)

@fforres
Copy link
Member Author

fforres commented Aug 28, 2018

The change in hexdex page is based in chapter page, so if navigator do not have JS enabled, then, chapter and hexdex do not work.

Aaaaaah, good point!
How do you feel about adding a <noscript> tag with the old content, and the provide the new and improved UI when JS is enabled?
https://www.w3schools.com/tags/tag_noscript.asp

@ogsilvaa
Copy link

Perfect, I will add more description on each image, it seems to me to place the administrators too. I will correct the height and you are right about the contrast only that it will be difficult to find a color that makes contrast to all better, change the effect.

'----

Perfecto, agregaré más descripción sobre cada imagen, me parece que colocar los administradores también. Voy a corregir la altura y tienes razón sobre el contraste solo que va a ser difícil encontrar un color que le haga contraste a todos mejor, cambiar el efecto.

@fforres
Copy link
Member Author

fforres commented Aug 28, 2018

@ogsilvaa
You merged #627?
I was under the impression you where going to fix the issues?
I think the changes pushed create accessibility issues we should not willingly create.

@ogsilvaa
Copy link

There are no more accessibility problems than the ones that already exist. The incidences do not suppose a major problem they are only visual. On the other hand, the page has a substantial improvement since it now shows all the badges already raised in the repo. I appreciate your comments and will make the visual corrections given, as well as the creation of new features. I will appreciate that you also accompany me in this. regards

'-----

No hay más problemas de accesibilidad que los ya existentes. Las incidencias no suponen un problema mayor solo son visuales. Por otra parte, la página tiene una mejora sustancial ya que ahora muestra todas las insignias ya subidas en el repo. Agradezco tus comentarios y se realizarán las correcciones visuales dadas, así como la creación de nuevas características. Agradeceré que también me acompañes en esto. Saludos

@ogsilvaa ogsilvaa mentioned this issue Aug 28, 2018
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

No branches or pull requests

2 participants