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

feat: create IconBase component, add script to create Icon.tsx file #548

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

RohitRaj011
Copy link
Contributor

@RohitRaj011 RohitRaj011 commented Jan 30, 2025

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Checklist

  • The title of the PR states what changed and the related issues number (used for the release note).
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas

@RohitRaj011 RohitRaj011 self-assigned this Jan 30, 2025
@RohitRaj011 RohitRaj011 marked this pull request as draft January 30, 2025 07:20
@RohitRaj011 RohitRaj011 marked this pull request as ready for review February 3, 2025 05:02
Base automatically changed from refactor/app-status-filter-button to develop February 4, 2025 09:53
import { Icon, IconName } from '../Icon'
import { RegistryIconProps } from './types'

const getRegistryIcon = (registryType: RegistryType): IconName => {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: can create an object instead

tooltipProps={{
alwaysShowTippyOnHover: true,
placement: 'top',
content: 'To fetch app status for Helm based deployments open the app detail page',
Copy link
Contributor

Choose a reason for hiding this comment

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

move to app status


<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">
<path d="M15.7501 11.25V6.375C15.7501 5.87772 15.9477 5.40081 16.2993 5.04917C16.6509 4.69754 17.1278 4.5 17.6251 4.5C18.1224 4.5 18.5993 4.69754 18.9509 5.04917C19.3026 5.40081 19.5001 5.87772 19.5001 6.375V14.25C19.5001 15.2349 19.3061 16.2102 18.9292 17.1201C18.5523 18.0301 17.9999 18.8569 17.3034 19.5533C16.607 20.2497 15.7802 20.8022 14.8702 21.1791C13.9603 21.556 12.985 21.75 12.0001 21.75C7.85798 21.75 6.00011 19.5 2.94681 13.0581C2.82345 12.8449 2.74333 12.6094 2.71102 12.3652C2.67871 12.1209 2.69485 11.8727 2.75851 11.6347C2.82217 11.3967 2.93211 11.1736 3.08205 10.9781C3.23198 10.7827 3.41897 10.6186 3.63232 10.4955C3.84568 10.3723 4.08122 10.2923 4.32548 10.2602C4.56974 10.2281 4.81793 10.2445 5.05587 10.3083C5.29381 10.3722 5.51683 10.4823 5.71218 10.6324C5.90754 10.7825 6.0714 10.9697 6.19441 11.1831L8.25011 14.7437V4.875C8.25011 4.37772 8.44766 3.90081 8.79929 3.54917C9.15092 3.19754 9.62783 3 10.1251 3C10.6224 3 11.0993 3.19754 11.4509 3.54917C11.8026 3.90081 12.0001 4.37772 12.0001 4.875V11.25" fill="var(--R500)" fill-opacity="0.12"/>
<path d="M15.7501 11.25V3.375C15.7501 2.87772 15.5526 2.40081 15.2009 2.04917C14.8493 1.69754 14.3724 1.5 13.8751 1.5C13.3778 1.5 12.9009 1.69754 12.5493 2.04917C12.1977 2.40081 12.0001 2.87772 12.0001 3.375V11.25" fill="var(--R500)" fill-opacity="0.12"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

let's pass the hex instead & use color prop. check across

@@ -0,0 +1,4 @@
<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

- limitations under the License.
-->

c<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
c<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">
<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">

</Tooltip>
)

export const IconBase = ({ name, iconMap, size = 16, tooltipProps, color, strokeWidth }: IconBaseProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Check with Utkarsh, stroke width should be a function of size instead of prop.

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.

2 participants