Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 3x3 + OH + BLD Relay and Team Factory icons #109

Merged
merged 2 commits into from
Sep 11, 2023
Merged

Conversation

dmint789
Copy link
Contributor

@dmint789 dmint789 commented Sep 8, 2023

This PR makes the following changes:

Add 3x3 + OH + BLD Relay icon (333-oh-bld-relay)
Add 3x3 + OH + BLD Team Relay icon (333-oh-bld-team-relay)
Add 3x3 Team Factory icon (333-team-factory). Similar idea to the Team-Blind icon.
Slightly tweak Team-Blind icon, making the circles a little smaller. The circles are consistent between Team Factory and Team Blind.

Previews:

Untitled

This solves part of #105

@dmint789
Copy link
Contributor Author

dmint789 commented Sep 8, 2023

I figured it made more sense to include the non-team version of 3x3 + OH + BLD relay icon too, since that's the less specific case, and then the team version simply builds on that icon, using the exact same 3 circles as in 3x3 BLD 3-man relay.

Copy link
Member

@jfly jfly left a comment

Choose a reason for hiding this comment

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

Team factory and team bld look good to me.

The relay icons look quite crowded to me, but I don't actually have a suggestion for fixing them.

Approved from my perspective, but I'd like to hear from another maintainer (@lgarron?) before merging.

@dmint789
Copy link
Contributor Author

dmint789 commented Sep 8, 2023

I just went with the same idea as Mini guildford. I actually think it looks alright, because it's quite clear what it represents.

@dmint789
Copy link
Contributor Author

dmint789 commented Sep 8, 2023

One option is to replace the OH cube with a single square, same as Mini Guildford does it

@dmint789
Copy link
Contributor Author

dmint789 commented Sep 8, 2023

@jfly I have made the aforementioned change, now it's the same as in Mini Guildford:

333-oh-bld-relay

@dmint789
Copy link
Contributor Author

dmint789 commented Sep 8, 2023

I would appreciate having a new NPM package version once this PR is merged. I would like to use all of the new icons.

@jfly
Copy link
Member

jfly commented Sep 8, 2023

I'll merge this (and do an npm release) early next week unless another maintainer gives feedback before then.

@dmint789
Copy link
Contributor Author

@jfly Could you deploy this today? I would like to add the new icons to my website :)

@jfly
Copy link
Member

jfly commented Sep 11, 2023

I'll merge this tomorrow morning unless someone chimes in before then. @lgarron last chance for feedback! (of course, we can always change these things in the future if want)

Copy link
Member

@lgarron lgarron left a comment

Choose a reason for hiding this comment

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

I'd love to find a better way to combine separate icons (maybe a cartouche?) at some point, but these look good to me for now!

@jfly jfly merged commit fc1347a into cubing:main Sep 11, 2023
1 check passed
@jfly
Copy link
Member

jfly commented Sep 11, 2023

@dmint789 version 1.0.9 (https://github.com/cubing/icons/releases/tag/v1.0.9) of @cubing/icons has been released to npm.

@dmint789
Copy link
Contributor Author

@jfly thank you! I've already updated my package :)

@lgarron
Copy link
Member

lgarron commented Sep 11, 2023

Did we not want to use underscores before releasing?

@jfly
Copy link
Member

jfly commented Sep 11, 2023

What are underscores?

@dmint789
Copy link
Contributor Author

@jfly this character: _
@lgarron well, we ended up sticking with dashes in both this PR and the Team-Blind one, to stick with the convention started by mirror cube.

@jfly
Copy link
Member

jfly commented Sep 11, 2023

Ahh sorry, I thought we were talking about release naming conventions or something.

Let's continue the discussion about naming conventions over on #108

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants