Conversation
panch1739
added
documentation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Rabbit Dashboard application and updates documentation for various input components to standardize terminology. Critical syntax errors were identified in the JavaScript file where unescaped single quotes in ASCII art strings will cause the script to fail. Additionally, several feedback points address violations of the UX writing guidelines, specifically regarding the removal of colons in labels and status messages to improve conciseness. A documentation update is also required to ensure consistent use of the term 'Helper text' instead of 'Feedback text'.
| @@ -0,0 +1,54 @@ | |||
| const variants = { | |||
| small: ['(\\_/)\n(\'.')\n(\" ) (\")', '(\\_/)\n(•_•)\n(> <)'], | |||
There was a problem hiding this comment.
This line contains a syntax error. The unescaped single quote in ('.') terminates the string literal prematurely, which will cause the script to fail. You should escape the single quotes or use double quotes for the string.
small: ['(\\_/)\n(\'.\')\n(\" ) (\")', '(\\_/)\n(•_•)\n(> <)'],| @@ -0,0 +1,54 @@ | |||
| const variants = { | |||
| small: ['(\\_/)\n(\'.')\n(\" ) (\")', '(\\_/)\n(•_•)\n(> <)'], | |||
| classic: [' (\\_/)\n (=\'.'=)\n (")_(")'], | |||
There was a problem hiding this comment.
This line contains a syntax error. The single quote in (='.'=) must be escaped to prevent the string from terminating early.
classic: [' (\\_/)\n (=\'.\')\n (")_(")'],| <h1>Rabbit Dashboard</h1> | ||
|
|
||
| <section class="controls"> | ||
| <label for="variant">Variant:</label> |
There was a problem hiding this comment.
According to the UX writing guidelines, colons should be avoided in labels.
| <label for="variant">Variant:</label> | |
| <label for="variant">Variant</label> |
References
- Avoid colons whenever possible (e.g., "User name" not "User name:") (link)
| function setVariant(name){ | ||
| const frames = variants[name]; | ||
| rabbitEl.textContent = Array.isArray(frames)? frames[0] : frames; | ||
| statusEl.textContent = `Status: showing ${name}`; |
| async function animate(name){ | ||
| const frames = variants[name]; | ||
| if(!frames || frames.length<2) return; | ||
| animating = true; animateBtn.textContent = 'Stop'; statusEl.textContent = 'Status: animating'; |
There was a problem hiding this comment.
Simplify the status message and remove the colon to adhere to UX guidelines.
animating = true; animateBtn.textContent = 'Stop'; statusEl.textContent = 'Animating';References
- Avoid colons whenever possible. (link)
| i++; await new Promise(r=>setTimeout(r,300)); | ||
| } | ||
| animateBtn.textContent = 'Animate hop'; | ||
| statusEl.textContent = `Status: idle (last ${name})`; |
There was a problem hiding this comment.
Remove the colon and the redundant Status prefix to follow UX writing standards.
statusEl.textContent = 'Idle (last ' + name + ')';References
- Avoid colons whenever possible. (link)
|  | ||
|
|
||
| > 1. Label (optional), 2. Pills, 3. Container, 4. Message (optional) | ||
| > 1. Label (optional), 2. Pills, 3. Container, 4. Feedback text (optional) |
There was a problem hiding this comment.
Use the term Helper text instead of Feedback text for the optional message below the container to maintain consistency with other input documentation.
| > 1. Label (optional), 2. Pills, 3. Container, 4. Feedback text (optional) | |
| > 1. Label (optional), 2. Pills, 3. Container, 4. Helper text (optional) |
References
- In documentation, prioritize conciseness over explaining the 'why' when the reason is considered obvious.
1 participant
This PR updates the images of the inputs based on the latest alignment.
Related to #1732
Please make sure your PR follows the contribution guidelines
https://github.com/siemens/element/blob/main/CONTRIBUTING.md. -->
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: