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.

Sign up for GitHub

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

Jump to bottom

Added ContextualHelp icon #315

Merged
merged 3 commits into from
Jun 6, 2023
Merged

Added ContextualHelp icon #315

merged 3 commits into from
Jun 6, 2023

Conversation

MikeKingdom
Copy link
Collaborator

Added the ContextualHelp icon requested by GLCP.

Fixes grommet/hpe-design-system#3361

@MikeKingdom MikeKingdom requested review from halocline and jcfilben May 30, 2023 20:32
jcfilben
jcfilben reviewed May 30, 2023
View reviewed changes
Copy link
Collaborator

@jcfilben jcfilben left a comment •
edited
Loading

Choose a reason for hiding this comment

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

PR looks good, just one thing I am noticing is the thickness of the lines on this icon doesn't match our other icons. Just wanted to confirm if this is intentional.
Here is the ContextualHelp icon next to the CatalogOption icon. From what I am seeing the other icons in grommet-icons have a similar line thickness to CatalogOption.
Screen Shot 2023-05-30 at 4 43 15 PM

@MikeKingdom
Copy link
Collaborator Author

PR looks good, just one thing I am noticing is the thickness of the lines on this icon doesn't match our other icons. Just wanted to confirm if this is intentional. Here is the ContextualHelp icon next to the CatalogOption icon. From what I am seeing the other icons in grommet-icons have a similar line thickness to CatalogOption. Screen Shot 2023-05-30 at 4 43 15 PM

Great question. I'll take a closer look. I think Gina originally started with the CatalogOption so I'll have to see what happened since then.

jcfilben
jcfilben reviewed Jun 1, 2023
View reviewed changes
Copy link
Collaborator

@jcfilben jcfilben left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Line weight on the book looks good, can we also adjust the line weight of the question mark? It looks like on other icons where we have that have stuff inside of them the line weight stays consistent (for example the lines within the article icon are all the same weight)
Screen Shot 2023-06-01 at 3 42 57 PM

@MikeKingdom
Copy link
Collaborator Author

I adjusted the weight on the question mark and made it a little bigger to better use the space and be more readable.
image

jcfilben
jcfilben approved these changes Jun 5, 2023
View reviewed changes
Copy link
Collaborator

@jcfilben jcfilben left a comment

Choose a reason for hiding this comment

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

Looks good!

@taysea taysea self-requested a review June 6, 2023 16:28
taysea
taysea approved these changes Jun 6, 2023
View reviewed changes
Copy link
Collaborator

@taysea taysea left a comment

Choose a reason for hiding this comment

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

LGTM.

@MikeKingdom MikeKingdom merged commit 45b2153 into master Jun 6, 2023
@MikeKingdom MikeKingdom deleted the contextual-help branch June 6, 2023 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcfilben jcfilben jcfilben approved these changes

@taysea taysea taysea approved these changes

@halocline halocline Awaiting requested review from halocline

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DS/Figma feature: New icon for GLCP Contextual Help
3 participants
@MikeKingdom @taysea @jcfilben