in AssignmentIconds replace <path>s with <circle>s where applicable #1575
Conversation
This file contains 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
ap-nri
force-pushed
the
hackday-real-circles
branch
from
0be7b2d to
922d149
Compare
ap-nri
requested review from
a team,
omnibs,
charbelrami and
bendansby
and removed request for
a team
ap-nri
force-pushed
the
hackday-real-circles
branch
2 times, most recently
from
477a5c4 to
25a273e
Compare
|
FYI #1573 is changing the daily writing icon |
17 tasks
bendansby
approved these changes
Dec 4, 2023
charbelrami
approved these changes
Dec 4, 2023
|
nice tysm @bcardiff I will add that to this |
ap-nri
force-pushed
the
hackday-real-circles
branch
from
fd09a03 to
1e91ac5
Compare
|
@NoRedInk/design I have updated this to
Once approved I will do the commit dropping the old icons. NB @charbelrami these are not pixel perfect! Several of the old icons did not have their circles touch the edges of the containing box ( Note for myself: you get all the SVG code as Markdown with $$('title').forEach(t => t.remove());
$$('[class]').forEach(x => x.removeAttribute('class'));
t3 = '```';
copy($$('tbody tr').map(row =>
`### ${row.innerText.trim()} \n${t3}svg\n${row.children[2].innerHTML}\n${t3}`
).join('\n\n')) |
|
@bendansby re-requesting but the only thing that changed was Daily Writing |
bendansby
approved these changes
Dec 5, 2023
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.
This is now ready to merge! All changes are approved, old versions of icons dropped, and updated
<svg>s are in this readme as code (and can be exported with some judicious Taco Bell programming).HOLD UP NO MERGING YETSo the checklist is:As of right now this has two versions of every applicable icon. When the icon name is suffixed with
Xthat's actually the original version of the icon using a path. I include those because I want design to be able to compare side by side. Once they approve, I will make a commit removing all of them leaving only the versions with<circle>s.🔧 Modifying a component
Context
Sometimes circular icons in in AssignmentIcons look unsharp around the outsides. This is because they do not use Circle or Ellipse elements, but Paths.
🖼️ What does this change look like?
You can see little pixel imperfections in this, not gonna lie. In the old versions a lot of the circles were slightly elliptical or did not touch the sides of their bounding box. Maybe that actually works better because the shape inside of the circle looks more centered in those circles, and I get that. Now the circles touch all four sides of the bounding rectangle.
You can definitely get a better feel with a live page. The preview lets you blow up the icons pretty big (turn on "Show names" to know which is which). I can also send the HTML page that I've screenshot below. There's no good way to serve it to you other than as an image. Stinks.
Rendered SVG code for each
Collapsed because it's a ton of code blocks that make this hard to read
startPrimary
assessment
practiceCircled
modules
writing
passageQuizCircled
planningDiagnosticCircled
quizCircled
unitDiagnosticCircled
quickWriteCircled
guidedDraftCircled
selfReviewCircled
peerReviewCircled
gradingAssistantCircled
dailyWritingCircled
Component completion checklist
nriDescription[x] If this is a new major version of the component, our team has stories created to upgrade all instances of the old component. Here are links to the stories: