FIX: Software update prompt fixes and improvements (PR #12648)

[martin]

GitHub

[CvX]

app/assets/javascripts/discourse/app/components/software-update-prompt.js

@@ -16,9 +17,10 @@ export default Component.extend({
     return getURL("/");
   },
 
-  isHidden: not("showPrompt"),
-
-  _timeoutHandler: null,
+  @discourseComputed("showPrompt")
+  isHidden(showPrompt) {
+    return !showPrompt ? "true" : "false";

@jjaffeux you mentioned that aria-hidden="false" is not recommended?

Would it be better to make this a “tagless” component and handle all this in the template? (using the new not helper) Something like:

<div class="software-update-prompt {{if showPrompt "require-software-refresh"}}" {{if (not showPrompt) "aria-hidden='true'"}}>

[jjaffeux]

Yes that would work I think. I had a quick talk with @martin this morning on what limitations he would face when trying to move to tagless.

[martin]

@CvX @jjaffeux I tried to move to tagless but I ran into issues rendering the aria-hidden attribute with if using the syntax provided. I get this error:

For aria-hidden, @jjaffeux said:

Also I think you should double check aria-hidden usage, AFAIK, only aria-hidden=“true” is valid and in the test I only see a check for element.getAttribute(“aria-hidden”) === “”

According to MDN Using the aria-hidden attribute - Accessibility | MDN aria-hidden="false" is a valid value and is actually the default. However, under best practices Using the aria-hidden attribute - Accessibility | MDN it says aria-hidden is not even needed if visibility: hidden is specified for the element, which it is.

I have now made this a tagless component and removed the aria-hidden attribute altogether.

[CvX]

:shipit: