Skip to content

chore(MOPRAT-841): Add ArrowheadUpCircleFillIcon#79

Merged
MrSltun merged 2 commits into
mainfrom
mrsltun/MOPRAT-841/add-arrowhead-up-filled-icon
May 16, 2025
Merged

chore(MOPRAT-841): Add ArrowheadUpCircleFillIcon#79
MrSltun merged 2 commits into
mainfrom
mrsltun/MOPRAT-841/add-arrowhead-up-filled-icon

Conversation

@MrSltun

@MrSltun MrSltun commented May 16, 2025

Copy link
Copy Markdown
Member

This PR resolves MOPRAT-841

Description

This PR adds a new icon ArrowheadUpCircleFillIcon, renamed from ArrowUpCircleFillIcon, so that it's not confused with the current Arrow icons we currently have in Artsy Icons (arrow icons in Palette Mobile are chevron icons in Artsy Icons)

But I'm open to different naming for the icon

New Icon

image

@MrSltun MrSltun requested review from anandaroop and Copilot May 16, 2025 15:14
@MrSltun MrSltun self-assigned this May 16, 2025

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

@artsy-peril artsy-peril Bot added Jira Synced Indicates that Peril has connected this PR to Jira Version: Minor A deploy for new features labels May 16, 2025

@anandaroop anandaroop left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

But I'm not sure about the naming question either…

We've lost the "circle" concept from the name — do you think ArrowheadUpFilledCircleIcon is too verbose?

I leave it up to you, whether you want design to weigh in or just go for it.

@MrSltun

MrSltun commented May 16, 2025

Copy link
Copy Markdown
Member Author

We've lost the "circle" concept from the name — do you think ArrowheadUpFilledCircleIcon is too verbose?

I don't think it's too verbose, I just mistakenly missed it 😅
I would suggest going with ArrowheadUpCircleFillIcon (after checking what we have in Artsy Icons), what do you think?

@MrSltun MrSltun requested a review from dzucconi May 16, 2025 15:31
@dzucconi

Copy link
Copy Markdown
Member

ArrowheadUpCircleFillIcon is good. Adheres to adjective order better :) (Determiner Opinion Size Age Shape Color Origin Material and Purpose) (Shape before color/style)

@MrSltun MrSltun changed the title chore(MOPRAT-841): Add ArrowheadUpFilledIcon chore(MOPRAT-841): Add ArrowheadUpCircleFillIcon May 16, 2025
@MrSltun MrSltun merged commit 8b59476 into main May 16, 2025
@artsyit

artsyit commented May 16, 2025

Copy link
Copy Markdown
Contributor

🚀 PR was released in v3.34.0 🚀

@MrSltun MrSltun deleted the mrsltun/MOPRAT-841/add-arrowhead-up-filled-icon branch July 11, 2025 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Jira Synced Indicates that Peril has connected this PR to Jira released Version: Minor A deploy for new features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants