-
Notifications
You must be signed in to change notification settings - Fork 28
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(core): export gosling svg logo #1038
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments. I'd export this function (and use it) as a React Component.
@@ -0,0 +1,31 @@ | |||
import React from 'react'; | |||
|
|||
export const getGoslingLogoSvg = (width = 20, height = 20) => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function returns JSX, so it requires React to use it (i.e., it is a React component). Rather than getGoslingLogoSvg
why not export GoslingLogo
component:
type GoslingLogoProps = {
width: number;
height: number;
}
export function GoslingLogo({ width = 20, height = 20 }: GoslingLogoProps) {
return <svg>...</svg>
}
I'd also include some JSDoc comments for GoslingLogoProps
to explain what width and height are (pixels, fractional units, etc).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If width and height should always be then same, then I'd go for just size
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import react from 'React';
import { GoslingLogo } from 'gosling.js';
function App() {
return <GoslingLogo size={20} />
}
Another option would be to have a button in the Gosling editor /website to copy the icon in different ways (svg "raw", or JSX): https://lucide.dev/icons/align-horizontal-distribute-end I personally like this option because you can copy the SVG directly into your project and style it however you like (with or without react). With the React component, you define how styling needs to happen. |
Ah, I like the idea of adding a button to copy the logo! I am now leaning toward adding that to the editor or probably our website. |
Can we include the SVG element of the Gosling logo image in the gosling.js library? This would make it easier for people to add a Gosling logo to their applications (e.g., "Powered by" labels). Helpful for some of our applications in the lab as well since we do not need to copy and paste the SVG code.
Change List
Checklist