Closed hannaeko closed 8 months ago
@blacksponge, have you considered #315 in this PR?
I have, but it is actually out of scope. I am not touching the results details in this one.
I do plan to improve it in the future, to also address the point Expand (+ highlight?) text in search result
that I have crossed out of the todo list for now.
I will review when this is no longer a draft.
I have deployed the version from this branch at https://zonemaster.fr/en (it is a few commits behind though)
I will review when this is no longer a draft.
This is now ready for review.
Do these changes make it easier to customize the look and feel? My colleague has asked for the CSS to be decoupled from the code to make it easier to change CSS. Will these changes make it easier or harder to change CSS?
For me it looks like the FAQ is integrated in HTML code. How do you expect translation to be done?
I changed language in the FAQ to Swedish in zonemaster.fr and after that whatever I do I end up in "file not found". Not even reloading helps.
I changed language in the FAQ to Swedish in zonemaster.fr and after that whatever I do I end up in "file not found". Not even reloading helps.
Only English and French are deployed on zonemaster.fr.
For me it looks like the FAQ is integrated in HTML code. How do you expect translation to be done?
Using the same translation system as all other strings in the GUI.
Do these changes make it easier to customize the look and feel? My colleague has asked for the CSS to be decoupled from the code to make it easier to change CSS. Will these changes make it easier or harder to change CSS?
No. It is still Angular, so no change on that side.
For me it looks like the FAQ is integrated in HTML code. How do you expect translation to be done?
Using the same translation system as all other strings in the GUI.
That works for strings, but maybe less good for texts. A reply is often more than one paragraph. I will do a test translation as part of reviewing. Do you see that it will be possible to migrate current translations into the new framework?
That works for strings, but maybe less good for texts.
I would agree but having it in markdown is very non optimal. It limits the document structure. For the kind of things that I have done it would either mean writing directly html or a custom markdown parser. Both would be hard to integrate within the current framework. And I am not please with either of the current solution, whether it is requesting a static html file to inject in the page or having a full angular app while all we really need is a static html file and 15 lines of javascript.
I will do a test translation as part of reviewing.
As said in the description I am also testing Crowdin for translation. If you want I can add you to my test project and you can test that as well.
Do you see that it will be possible to migrate current translations into the new framework?
Partial manual migration is possible, there have been some small changes in a few strings but only a minority.
I changed language in the FAQ to Swedish in zonemaster.fr and after that whatever I do I end up in "file not found". Not even reloading helps.
Only English and French are deployed on zonemaster.fr.
My mistake, it looks like I have overridden the configuration in the last deployment, it should not have displayed Swedish.
It is not possible for me to view zonemaster.fr on some platforms. It seems like if the preferred language is "sv" it will end up trying to load a page not existing. Can that be changed?
As far as I can see, the pictures on the first page are hard-coded into the GUI. I think that is wrong. If there should be room for picture, which could be fine, it should be easy to exchange those for other pictures that fits the frame that the GUI belongs to. The pictures, if any, should be se in app.config.json.
The PR also changes the texts, as far as I can see. It should be separated so that we decide on form and text in different steps. That will also make it easier to see what texts have changed for the translation work.
The PR also changes the texts, as far as I can see. It should be separated so that we decide on form and text in different steps. That will also make it easier to see what texts have changed for the translation work.
I changed some of the text indeed, I can move the FAQ changes outside of this PR but I will keep the "About Zonemaster" in this one.
As far as I can see, the pictures on the first page are hard-coded into the GUI. I think that is wrong. If there should be room for picture, which could be fine, it should be easy to exchange those for other pictures that fits the frame that the GUI belongs to. The pictures, if any, should be se in app.config.json.
I can do that.
It is not possible for me to view zonemaster.fr on some platforms. It seems like if the preferred language is "sv" it will end up trying to load a page not existing. Can that be changed?
I changed the reverse proxy configuration, can you check if it is better.
As far as I can see, the pictures on the first page are hard-coded into the GUI. I think that is wrong. If there should be room for picture, which could be fine, it should be easy to exchange those for other pictures that fits the frame that the GUI belongs to. The pictures, if any, should be se in app.config.json.
I can do that.
As the pictures are definied in CSS and that the style can be overridden with #439 I do not see the need to add a specific configuration.
The PR also changes the texts, as far as I can see. It should be separated so that we decide on form and text in different steps. That will also make it easier to see what texts have changed for the translation work.
I changed some of the text indeed, I can move the FAQ changes outside of this PR but I will keep the "About Zonemaster" in this one.
The FAQ changes in this PR have been reverted and moved to #443.
Things that I have notice when zooming at 200% (text only on firefox)
Some potential improvements, could be fixed if the behaviour followed the one defined for normal zoom (media query on screen size)
Contrast issue
And maybe a good idea to adjust the bottom line color on active menu link to reach a contrast ratio of 3? (cf SC 1.4.11 and associated understanding document) Also need to double check switch (inside / outside color in both states).
Other things to look at:
So as I have noticed some issues with the current UI and this PR, I will fix the small issues here and properly open new issues for the more complex ones. Otherwise this PR will never be merged.
Okay I am done adding more stuff to this PR. I will open issues for the remaining things.
Underlining signals link on the web, but the underlined text is not a link (there are more such cases):
It is recommended not to use underlining when it is not a link.
If you select "History" you get a list of tests. The items look like links, but when you click on them nothing appearing happens (things are changed in the background, but that is hard to see). -- Discussed in #447 and #242. Not in this PR.
What is the license and copyright of the pictures on the entry page? Is it stated somewhere?
They are hard coded in src/app/components/domain/domain.component.css. Is that the only place to change if you want to use something else?
We should rather refer to https://github.com/zonemaster/zonemaster/blob/master/docs/contact-and-mailing-lists.md then linking to the list server.
tests have been updated, I just need to update the screenshots for this to be fully ready
* The default contact address has been changed to zonemaster@
The problem is that it will lead to a lot of spam. The contact@ should work in the sense that you get a pointer to the page about mailing lists. I have asked IT why it does not. The risk is that we will have to do the same thing with the zonemaster@ address.
The problem is that it will lead to a lot of spam. The contact@ should work in the sense that you get a pointer to the page about mailing lists. I have asked IT why it does not. The risk is that we will have to do the same thing with the zonemaster@ address.
Is it working though ? Because from yesterday's tests it did not seem to work. And putting a non working email address is kind of misleading.
The problem is that it will lead to a lot of spam. The contact@ should work in the sense that you get a pointer to the page about mailing lists. I have asked IT why it does not. The risk is that we will have to do the same thing with the zonemaster@ address.
Is it working though ? Because from yesterday's tests it did not seem to work. And putting a non working email address is kind of misleading.
It is actually working when I just tested. In the working group we agreed to have it working like this, and using the adresse as the outward facing email address.
I got the following:
Date: Fri, 8 Dec 2023 09:03:41 +0000
From: contact zonemaster <contact@zonemaster.net>
To: Mats Dufberg
Subject: Automatic reply: Testmeddelande contact
This mail address has been closed to excessive amount of spam messages.
Please follow the link below for how to contact the Zonemaster project.
Cette adresse e-mail a été fermée suite à un nombre excessif de messages
indésirables. Veuillez suivre le lien ci-dessous pour savoir comment
contacter le projet Zonemaster.
https://github.com/zonemaster/zonemaster/blob/master/docs/contact-and-mailing-lists.md
Thank you/Merci
Zonemaster Project
https://github.com/zonemaster/zonemaster/
https://zonemaster.net/
@blacksponge, could you test sending an email to contact@?
- The line under the h2 has been removed
Yes, I could see that. That is better.
could you test sending an email to contact@?
I just sent a email to contact@zonemaster.net (from both my personal and work email) and did not receive any response yet.
The tests are now passing. What should I do regarding the email address?
The tests are now passing. What should I do regarding the email address?
Please keep contact@. I will make sure it works for you anyone as for me. Please send the mail addresses you sent from to my email.
v2023.2 Release Testing
The "How to test" section was not done so I came up with my own testing, keeping in mind the "Changes" section of this PR when browsing:
(/run-test
, /result
and /faq
) using every functionalities in each page.I didn't find any obvious issues.
Purpose
Try to improve the overall look and feel and accessibility of the GUI.
Context
Changes
Highlights :
Also I took this oportunity to test crowdin.com integration, https://crowdin.com/project/zonemaster-test.My todo lists
Things I want to do (in this PR) - [x] Make "about" lines shorter - [x] Improve contrast between active and not active navigation links - [x] Make color usage more uniform - [x] disabled primary button color - [ ] secondary button color -- change or keep ? - [x] lighten danger and secondary button on hover instead of darkening them - [x] Try to improve "about" information visualization in "about" text (use markup) - [x] About zonemaster - [x] ~~About DNS~~ need to change the text, not doing this now - [x] Improve accessibility of advanced option form: - [x] make labels visible - [x] use a fieldset to group NS and DS inputs - [x] Improve accessibility of result filtering form - [x] maybe replace links that look like buttons with checkboxes and labels in fieldset - [x] Move help / severity level info out of tooltips - [x] Improve document outline - [x] replace h4/h5 with h2/h3 in about section - [x] Add section + headings for form / result in result page - [x] Improve "program version" (at least make the "program version" thing a link so it is an actionable item) - [x] ~~Expand (+ highlight?) text in search result~~ not in this PR - [x] Change FAQ layout - [ ] add "expand" button for each section -- surprisingly hard to do, need more thoughts Things that I broke and need to fix - [x] fix advanced form NS and DS inputs wrapping for medium / small screens - [x] main heading for result page - [x] fix message banner - [x] Click on faq entry update url fragment - [x] change of url fragment open faq entry - [x] fix link in info alert in advanced form - [x] i18n (french translation done) - [x] fix advanced switch (transition too long for focus outline, colors don't match other secondary buttons)How to test this PR
A live version is deployed at https://zonemaster.fr
// TODO