load material icons, rank from one, gate the skill row - #206
Open
nmbradley wants to merge 3 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four fixes to the adopted roll template.
The word "adjust" instead of a bullseye
The design replaces the Target/Damage label with a Material ligature:
Nothing imported that family, so the literal ligature name rendered. Material Symbols is a variable font and cannot come through Roll20's CSS-1 import, which the chat parser requires — so legacy
Material Iconsis added to the existing import instead. The SCSS already lists it as the fallback, so the declaration is unchanged:An empty verdict pill on cards that have no verdict
RANKSstarted at 0, and a card that sets no rank at all — an alert, a damage roll, an armour-destroyed notice — leaves the placeholder reading0. The verdict block testingrank 0matched it and printed an empty Critical Failure pill.Ranks now run 1–4, so "no rank" matches nothing and those cards show no verdict at all.
The Skill row reading "0"
Same root cause one field over.
{{#computed::skill}}is a section test, which reads the original roll — always truthy for a[[0]]placeholder — so the row rendered on every card, printing the placeholder where no skill was set. It is now gated by ahasskillflag tested withrollTotal, the same patternhasnotesalready uses.Smaller title, and one real defect behind it
The title slab now takes
--ms-text-lgrather than--ms-text-xl.While changing it: the adoption had left three separate
.sheet-ms-rollblocks overriding one another, so the height and padding that actually applied were whichever happened to come last —height: autobeat the design's fixed--ms-roll-h, which is part of why the advantage layout looked wrong. Merged into one block that keeps the intended fixed height.345 tests, tsc, eslint, stylelint clean.
Follow-up: the roll box
--ms-roll-h: var(--ms-roll-w)rather thanaspect-ratio, because the target footer is positioned bycalc(var(--ms-roll-h) + …)and has to follow the box — anaspect-ratiowith no explicit height would have left the footer floating at the old offset.grid-area: 1 / 1) with the spent one absolutely positioned into the corner, where it collided with the target readout. They now flow as rows, ordered so the counted die is first and the struck-out one sits under it.