Correct counting problem with hidden annotations #1734
Conversation
Browser should really make this a pointer though. Odd
When there are no error it still had to choose between 1 or many. 0 wasn't an option
…hidden' span is empty.
MrKickkiller
changed the title
| @@ -2,6 +2,10 @@ | |||
|
|
|||
| .code-table { | |||
|
|
|||
| #annotations-were-hidden a { | |||
| cursor: pointer; | |||
There was a problem hiding this comment.
If you add a href="#" to the a element, you don't need any styling for it.
There was a problem hiding this comment.
I believe I have tried this, but I would have had to insert some more javascript to stop the page from reloading since clicking a <a href="#"></a> will navigate to the same page.
There was a problem hiding this comment.
Just return false; at the end of your event handler or call event.preventDefault();
There was a problem hiding this comment.
+1 for using href without css pointer
| @@ -151,6 +151,10 @@ export class CodeListing { | |||
|
|
|||
| public createHiddenMessage(count: number): HTMLSpanElement { | |||
| const span = document.createElement("span"); | |||
| if (count === 0) { | |||
There was a problem hiding this comment.
it feels like this method shouldn't be called if there is no message to show.
Don't call method if error count == 0
| const spanText: Text = document.createTextNode(I18n.t("js.annotation.were_hidden.first") + " "); | ||
| span.appendChild(spanText); | ||
|
|
||
| const link: HTMLAnchorElement = document.createElement("a"); | ||
| const data: string = I18n.t(`js.annotation.were_hidden.second.${count > 1 ? "plural" : "single"}`).replace(/{count}/g, String(count)); | ||
| const linkText: Text = document.createTextNode(data); | ||
| link.href = "#"; | ||
| link.addEventListener("click", function (ev) { | ||
| ev.preventDefault(); |
There was a problem hiding this comment.
Where is the event handler for expanding all annotations? You should add this there.
There was a problem hiding this comment.
It is already attached here, giving the user a bit of leniency about where to click.
There was a problem hiding this comment.
Do we want this? The whole point of a link is to make users know that it's clickable, so I don't think we need clickability outside of the link. @bmesuere Do you have an opinion on this?
There was a problem hiding this comment.
only make link clickable, it's what users expect
| @@ -51,7 +51,7 @@ en: | |||
| hidden: Hide correct tests | |||
| annotations: | |||
| title: Annotations | |||
| show_all: Show annotations | |||
| show_all: Show all annotations | |||
| hide_all: Hide annotations | |||
There was a problem hiding this comment.
| hide_all: Hide annotations | |
| hide_all: Hide all annotations |
| @@ -51,7 +51,7 @@ nl: | |||
| hidden: Verberg correcte tests | |||
| annotations: | |||
| title: Annotaties | |||
| show_all: Toon annotaties | |||
| show_all: Toon alle annotaties | |||
| hide_all: Verberg annotaties | |||
There was a problem hiding this comment.
| hide_all: Verberg annotaties | |
| hide_all: Verberg alle annotaties |
Correct counting problem with hidden annotations
This pull request fixes the issues laid out in #1733
Count was incorrect due to only checking if > 1 when choosing 1 or many.
Added specific css to make the link portion a cursor: pointer
Locale was modified to be "Show all annotations" & "Toon alle annotaties"
Tests were added
Closes #1733 .