Development: Redesign the rating card - #2574
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 12 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Development: Redesign the rating card
This reverts commit 9204bf2.
# Conflicts: # src/main/webapp/app/shared/components/molecules/rating-section/rating-section.html
|
🤖 No OpenAPI or client changes needed. |
|
📊 Client Test Coverage Too Low 🔍 View coverage locally: pnpm run test:ci
open build/test-results/vitest/coverage/index.html🌐 View coverage from GitHub: |
|
📊 Client Test Coverage Too Low 🔍 View coverage locally: pnpm run test:ci
open build/test-results/vitest/coverage/index.html🌐 View coverage from GitHub: |
az108
left a comment
There was a problem hiding this comment.
While testing i noticed that we should maybe bundle the bewertungs section, having it in two lines like this can be misleading and looks like we have given these ratings, also i would maybe put the blue background away and give it a hover effect in the colour which the pill will have after its selected. This could reduce the confusion. And did you forget adding a clickable hover effect 👀
|
📊 Client Test Coverage Too Low 🔍 View coverage locally: pnpm run test:ci
open build/test-results/vitest/coverage/index.html🌐 View coverage from GitHub: |
|
📊 Client Test Coverage Too Low 🔍 View coverage locally: pnpm run test:ci
open build/test-results/vitest/coverage/index.html🌐 View coverage from GitHub: |
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
# Conflicts: # src/main/webapp/i18n/de/button.json # src/main/webapp/i18n/en/button.json
The excellent and poor chips became unreadable once selected. A selected chip is filled with its rating colour and labelled with the matching text-on-* token. Those tokens flip with the colour scheme, near-black in light and white in dark, while the fills stay the same shades in both. So each fill ends up paired with a foreground that suits only one of the two schemes. Measured against WCAG AA, in light mode very_bad sat at 2.80:1, very_good at 3.47:1 and bad at 4.39:1; in dark mode neutral sat at 1.65:1 and good at 3.70:1. The two reported are the worst of the five, but three others were failing too, and no single foreground fixes all of them: white is unreadable on the yellow. The foreground is now chosen per fill from the two fixed base tokens, which do not move with the scheme, since the fills do not either. White on the dark red and dark green, black on the yellow and mid green. The lowest pairing is now 4.72:1 and all five clear 4.5:1 in both schemes. Verified the generated utilities resolve in the compiled stylesheet, and updated the class expectations in the rating spec. Verified: 2010 client tests, typecheck, eslint with no errors, a11y lint, the production build and prettier. Co-Authored-By: Claude <noreply@anthropic.com>
Follow-up to the contrast fix: the label is black on every chip now, rather than switching between black and white depending on which fill it sits on. That needed lighter fills. The default/hover/active triad is too dark at the ends for a black label, and too dark for a white one on the yellow, which is why no single label colour worked with it. The chips now use their own five-step scale, light enough that a black label clears 4.5:1 on all of them and in both colour schemes: 5.68 on very bad, 9.19 on bad, 13.86 on neutral, 11.52 on good and 5.68 on very good. The two ends match, so the scale still reads as symmetric. The steps are declared as tokens next to the existing purpose-built ones, with the measured ratio recorded per step so the next person changing a shade can see what the number has to stay above. Worth noting for the review: the dark-scheme text.on* values this PR flips from black to white are also the inverseColor for every PrimeNG severity component, not just these chips. The chips no longer depend on them, but a warn-severity button or tag in dark mode is now white on amber, which is the 1.65:1 case. That is untouched here and wants checking separately. Verified the tokens and utilities resolve in the compiled stylesheet. Verified: 2010 client tests, typecheck, eslint with no errors, a11y lint, the production build and prettier. Co-Authored-By: Claude <noreply@anthropic.com>
Moves the poor chip from danger.500 to danger.400, taking its label contrast from 5.68:1 to 7.06:1 while staying clearly apart from fair. Excellent is left alone for now. The palette has no step between success.500 and success.600, and moving it to 500 puts it within a colour difference of about 10 of good, which is close enough that the two chips read as the same green. Trading the good/excellent distinction for contrast on a chip that already clears the minimum did not seem worth doing without asking. Co-Authored-By: Claude <noreply@anthropic.com>
Excellent moves from success.600 to success.500 and good from success.400 to success.300, so both greens lighten together. Moving excellent on its own would have left it about a colour difference of 10 from good, close enough that the two chips read as one green. The scale now sits between 7.06:1 and 13.86:1 against its black label, up from a worst case of 5.68:1, and every neighbouring pair stays at least 18 apart, so the five steps remain tellable apart. The recorded ratio next to each token has been updated to match. Verified the tokens resolve in the compiled stylesheet. Verified: 2010 client tests, typecheck, eslint with no errors, a11y lint, the production build and prettier. Co-Authored-By: Claude <noreply@anthropic.com>
Five stars that fill up to the chosen value, with the word for that value underneath. The stored scale is untouched: -2 to +2 is mapped to one to five stars for display only, so none of the seven places using this component change. The pills made colour carry the meaning, which is why the label kept colliding with the fill and took three attempts to get readable. Stars carry it by count instead, so filled against empty is legible whatever the scheme, and the word underneath says which step it is rather than leaving it to be inferred from a shade. The per-step colour tokens are gone with them. Interaction follows the radio group pattern: arrow keys move between steps, Home and End jump to the ends, and a roving tabindex keeps the row to a single tab stop rather than one per star. Picking the current star again clears the rating, so "not rated yet" stays reachable. The keydown handler sits on the stars rather than the group, since the group is not focusable and the a11y lint rightly objects to handlers on elements that cannot receive focus. Stars use the primary colour, which holds 5.13:1 against a light background and 3.45:1 against a dark one, clearing the 3:1 that non-text contrast asks for. Gold was the first instinct but only manages 1.51:1 on white. Read-only rows keep the same stars with an aria-label naming the rating, and the label beside them. Verified: 2014 client tests, typecheck, eslint with no errors, a11y lint, the production build and prettier. Co-Authored-By: Claude <noreply@anthropic.com>
The stars take the red to green scale again, applied to the whole row rather than to one chip: one star is red, three amber, five green. Because the colour now paints a shape instead of sitting behind a label, it only has to clear the 3:1 non-text minimum instead of 4.5:1, which is what makes the scale usable again. Each step uses the shade that clears 3:1 against both a light and a dark background. The middle step is a deep amber rather than a bright gold, since gold only reaches 2.4:1 on white. The measured pair is recorded next to each token. Hovering a star previews it: the row fills to that step in that step's colour and the word underneath fills in, and both fall back to the stored rating when the pointer leaves. The stored value is untouched until a click, and aria-checked keeps tracking the real rating rather than the preview. Keyboard focus previews the same way, so the two do not disagree. Verified the tokens and utilities resolve in the compiled stylesheet. Verified: 2024 client tests, typecheck, eslint with no errors, a11y lint, the production build and prettier. Co-Authored-By: Claude <noreply@anthropic.com>
…verage Two places show a star rating: the input on the application detail page, which moves in whole steps, and the average on the application cards, which can be fractional and shows half stars. They had separate colour logic, so they could drift, and the card also set its colour through an inline style. The step colours now live in one function that both call, alongside the existing Likert conversion. The card passes its average and the input passes its star count, and rounding to the nearest step keeps them agreeing: a 4.0 average is the same green as a four star rating. The empty stars share a colour too, so the card no longer uses a paler grey than the input. The inline style is gone with it. The middle step is a bright gold now rather than the deep amber. It reaches 1.65:1 on a light background, short of the 3:1 that non-text contrast asks for. That is a deliberate trade: the number of filled stars and the word beside them carry the rating, the colour only reinforces it, and every gold that clears the bar stops reading as gold. The reds and greens still clear it on both backgrounds. Verified the tokens and utilities resolve in the compiled stylesheet, and checked both views map the same values to the same colours. Verified: 2037 client tests, typecheck, eslint with no errors, a11y lint, the production build and prettier. Co-Authored-By: Claude <noreply@anthropic.com>
Adds the spec the component never had, covering the filled and half star counts across whole and fractional values, the boundary just under the half point, the unrated state, maxStars, the shared step colours, the empty star colour, the formatted value, showValue and the three sizes. Writing it turned up a real defect. The template renders star-half-stroke for a fractional rating, but that icon was never added to the registered set, only faStar was. Any average landing on a half star, which is every average from an even split of reviewers, hit "Could not find icon with iconName=star-half-stroke" and rendered nothing in its place. The icon is registered now. Verified: 2060 client tests, typecheck, eslint with no errors, a11y lint, the production build and prettier. Co-Authored-By: Claude <noreply@anthropic.com>
The rating section and the comment section each fetched the rating overview for the same application, so opening a review issued two identical requests. It also meant the two could disagree: changing your rating refreshed the rating section but left your comment showing the old score until a reload. The rating section already owned the rating state and refetched it after every write, so it now emits what it loaded and the review page passes that down to the comment section. Removing the second fetch left the loadCommentRatings error string unused, so it is gone from both locales. On the interview page the rating block also rendered the evaluation page's generic label below the stars, which both repeated the instruction above them and spoke about applications rather than interviews. The fixed 40px height it sat in was too short for the star scale and its word label, so both are removed and the component sizes itself. Co-Authored-By: Claude <noreply@anthropic.com>
The comment section paired each comment with a rating by comparing display names, so two reviewers called "Max Mustermann" would show each other's score. Names are not unique and never were, and the client had nothing else to match on: neither the comment nor the rating carried the id of the user behind it. Both DTOs now expose one, named after the flat variants that already do this: RatingDTO gains fromUserId and InternalCommentDTO gains authorUserId. The client keys the lookup on those instead. This also makes the ratings distinct on the server. RatingOverviewDTO holds a Set, so before this two reviewers sharing a name who happened to give the same score collapsed into a single entry. Co-Authored-By: Claude <noreply@anthropic.com>
Checklist
General
Server
Client
Motivation and Context
Closes #1519
Ratings were shown as a row of coloured pills, one per step, with the label written inside the pill. That put the meaning on the colour and the text on top of it, which left several steps unreadable, and it gave no sense of the scale's shape: nothing said whether a rating sat near the top or the bottom without reading the word.
Comments also showed no rating, so a reviewer's reasoning and their score were never visible together.
Description
Rating input
Average display
star-half-strokewas rendered for fractional averages but had never been added to the registered icon set, so any average landing on a half star rendered nothing in that position. The icon is registered now.Colours
Comments
RatingDTOandInternalCommentDTOeach gained one (fromUserIdandauthorUserId, named after the flat variants that already carry it), because the pairing previously compared display names and two reviewers sharing a name would show each other's score. It also makes the ratings distinct on the server, whereRatingOverviewDTOholds aSetthat used to collapse same-name reviewers who gave the same score.Interview page
Also in this PR
text.onSuccess,onDanger,onWarnandonInfochange from black to white. These are theinverseColorfor every PrimeNG severity component, not only the ratings, so severity buttons, tags and badges are affected in dark mode. Worth a look during review; a warn severity tag is now white on amber.Steps for Testing
Prerequisites:
Steps:
Review Progress
Code Review
Manual Tests
Screenshots
Test Coverage
Client
Server
Last updated: 2026-07-26 19:49:37 UTC