-
Notifications
You must be signed in to change notification settings - Fork 8
Conversation
- removed logic to public and internal view since now we will have same view for internal/public users
…y the green plus button in By Organizations page
… pages ( visible when Tab into focus)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
I'm going to check with @JuliannaR on the "noscript" aria-label. That said that can be adjusted in another PR as need be.
@@ -15,7 +15,7 @@ | |||
{% set GTM_key = "GOOGLE_TAG_MANAGER" | fetch_env() %} | |||
{% if GTM_key %} | |||
<!-- Google Tag Manager (noscript) --> | |||
<noscript><iframe src="https://www.googletagmanager.com/ns.html?id={{ GTM_key }}" | |||
<noscript role="application" aria-label="script for google tag manager"><iframe src="https://www.googletagmanager.com/ns.html?id={{ GTM_key }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JuliannaR can I get your insights on best practice here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the content informative? It looks like it. I have a feeling that the label might not get read.
It can be an option for user agents that don't support JavaScript depending on the approach however I think we'd need to test if its a useful fallback.
You could provide a functional alternative in the noscript similar to you have done and we test it, however this might cause an issue re: role="application" aria-label="". I can check it out for you.
* Add the utf-8 byte order marker to simplify issues with loading to Excel * Brought tests suites inline with models.py, handle the utf-8 BOM, and expect bytes over the wire. * Whoops. params where they should have been. * Minor changes to cache invalidation to get rid of write access rqmt. * ugh tests. * get_cache should be type-hinting a str return, not bool. Also, I was returning both a datetime, or a str. Whoops. * sigh. tests. remember the tests. * - removed Beta banner - removed Bold links in some pages - add Terms and Conditions in footers * - removed temporary Google Analytics - add Content Security Policy on header - moved some inline javascript call to a external file * forgot one inline onclick javascript * - implemented a whitelist for report names that can be call via the app URL. for now : only one report name is allowed : compliance * - forgot one file * build package for public app * fix syntax errors * fire new job names * added logic to only display the donut for Public users * forgot to remove bold for links for modal (How to read this table?) * removed some unwanted space * put back Beta Banner * Minor tweaks to config to enable usage of Azure Managed Service Identities in combination with Azure KeyVault. * this time with updated req's * local ci would be great when you're sleep deprived. * removed secret name out of code * Removed headers due to duplication.. The upstream servers are also placing these headers, so removing from here. * Security Update: pyyaml bump to pull in safe_load Fixes this yaml/pyyaml#74. Note we were already using safe_load. * Security Update: pyyaml version bump yaml/pyyaml#74 * Paginate scroll to top * add semi-colon * - Implementation of Google Tag Manager GTM ID is stored in Environment variable called GOOGLE_TAG_MANAGER * fix typo * fix data-domain, can't use comma to enclose value, break if value have comma in domain name * removed CSP policies from HTML header. CSP is now implemented on Nginx server. * - some cleanup before merge to Master branch * - to fix Alerts from LGTM * Compatibility with kubernetes (cds-snc#127) * Modification for deploying on k8s * Small fix on dockerfile * Added CI workflow file * Ignore pip pinning in CI * defer datatable render (cds-snc#129) * Changed worker type and worker amount (cds-snc#130) * Added PR review app configuration; * Actually hit the right container * Take 2 * Upgraded deps (cds-snc#132) Bump dependencies for pymongo and flask_pymongo. Fixes time based connection issues. * Task default organizations (cds-snc#136) * - set default view to Organizations instead of Domains - removed logic to public and internal view since now we will have same view for internal/public users * - fix some accessibilities issues * - put back role=row for TR. If not present, Mobile view doesnt display the green plus button in By Organizations page * - for Accessibility : implement "Skip to main content" link at top of pages ( visible when Tab into focus) * update content for the Guidance page (cds-snc#137)
Hi,
Thanks,
Saya