-
Notifications
You must be signed in to change notification settings - Fork 61
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
[@svelteui/core]: Add IconRenderer to various components that support HTMLOrSVGElement
icon prop
#223
[@svelteui/core]: Add IconRenderer to various components that support HTMLOrSVGElement
icon prop
#223
Conversation
d1e44bf
to
2e179a6
Compare
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.
Hi, thank you for this contribution. As a starting point in the review, could you revert all the changes made to .npmrc
, package-lock
and all tsconfig.json
files? So that it is easier to review
2e179a6
to
8fb552a
Compare
done! |
There is some syntax errors happening in the CI. I'll review this as soon as possible, but also pinging @Brisklemonade to check this out |
hi @BeeMargarida @Brisklemonade any update on getting this reviewed? |
Sorry for taking so long, I'm in the process of reviewing it, it has not been forgotten. |
8fb552a
to
ecc1685
Compare
got those changes sorted 👍🏻 |
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.
Thank you so much for this contribution!! 🙌 Sorry for taking so long to review.
packages/svelteui-core/src/components/Menu/MenuItem/MenuItem.svelte
Outdated
Show resolved
Hide resolved
packages/svelteui-core/src/components/IconRenderer/IconRenderer.stories.svelte
Outdated
Show resolved
Hide resolved
packages/svelteui-core/src/components/Notification/Notification.svelte
Outdated
Show resolved
Hide resolved
color={iconProps.color} | ||
class={classes.icon} | ||
/> | ||
<IconRenderer {icon} className={classes.icon} {...iconProps} iconSize={16} /> |
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.
There is some weird misalignment going with the icon, but I think that problem existed before, will have to fix in another PR (just leaving here to remind me, please ignore me)
packages/svelteui-core/src/components/IconRenderer/IconRenderer.stories.svelte
Show resolved
Hide resolved
ecc1685
to
5efd7d5
Compare
Thank you for your contribution! 🙌 |
not a problem! is there an estimate when the next minor will be released? my team want to use this in a project is all 😄 |
Great! I'm thinking of making a new release, it will probably be done Sunday this week |
* [docs]: added missing installation check boxes and fixed typo * [core]: fixed typos * [core]: Yarn and Storybook migration (#160) * [migration]: all packages on kit-next.377 and use storybook template * [@svelteui/core]: configuring story book (viewport modifications and set up) * [@svelteui/core]: moved lib contents to src * [@svelteui/core]: fixed stories for button * [core]: fixed all eslintrcs * [core]: started theme setup for core package * [core]: storybook working with dark and light theme * [core]: revert some changes and remove logs * [core]: remove unused logic * [core] yarn migration (#157) * [core]: migrate to yarn * [docs]: update contribution guide to include yarn * [core]: modify github pipelines to use yarn * [core]: format and lint code * [core]: update yarn.lock (#161) Co-authored-by: BeeMargarida <ams-amargarida@hotmail.com> * [@svelteui/core]: support number as size in Button (#165) * [@svelteui/core]: fix missing prop type in NumberInput * [@svelteui/core]: use new prop approach in all components * [@svelteui/core]: remove onMouseEnter and onMouseLeave prop and use dispatch events instead * [@svelteui/core]: simplify prop approach in Button * [@svelteui/core]: format code - yarn format result * [@svelteui/core]: revert yarn format * [@svelteui/core]: remove BrowserRender, ServerRender and Fragment component (#167) * [core]: normalize scripts with : and udpdate readmes (#169) * [core]: sorted package jsons * [core]: updated docs and core dev scripts * [@svelteui/core]: Export all components prop types (#173) * [@svelteui/core]: export all component types instead of the whole styles * [core]: yarn lock * [core]: linting + format all package.json * [@svelteui/core]: fix select autocomplete logic that showed previous option on reload (#177) * [@svelteui/core]: fix Tabs bug where events inside tab content were not propagated due to node handling (#180) * [@svelteui/prism]: fix indentation problem on the first line (#181) * [@svelteui/prism]: fix indentation problem on the first line * [core]: remove package.json from prettier * [docs]: fix docs dev command, add docs node_modules to clean:all command and update tsconfig of core * [@svelteui/demos]: remove unecessary code from demos code examples (#182) * [@svelteui/core]: updated tsconfig * [@svelteuidev/core] Menu custom control (#184) * [@svelteui/core]: support for custom control in Menu * [@svelteui/core]: expose menu functions for opening/closing/toggling the menu state outside the component * [@svelteui/demos]: demo for Menu custom control * [docs]: add custom menu control examples to docs * [docs]: add missing period * [core]: remove flacky test * [@svelteui/core]: add role and aria info to custom menu control * [@svelteui/core]: added menu story * [core]: syncpack was failing script * [docs]: added modal clarification and removed banner * [core]: changed publish scripts due to unknown npm bug * [@svelteui/core]: menu story * [core]: fixed peerdeps versioning issues * [release]: 0.7.2 * [core]: fixed dev:core script * [@svelteui/composables]: migrated directory structure from the use of lib * [@svelteui/composables]: removed storybook config * [core]: fixed package json * [core]: changed yarn install to yarn in workflow * [core]: Storybook for core, composables and dates packages (#190) * [core]: removed unused stories from packages * [core]: moved storybook configuration to configuration folder and updated paths * [core]: working storybook that shows core stories * [core]: simple use-click-outside story for testing purposes * [core]: delete old storybook folders * [core]: stabilize storybook config * [@svelteui/composables]: change to * [@svelteui/dates]: change to * [core]: stories for dates, core and composables and simple example stories * [core]: format and lint fixes * [@svelteui/prism]: ignore line for prettier in prism * [@svelteui/composables]: small fix in alias * [@svelteui/composables]: small fixes to alias * [core]: bump vitest, cleanup dependencies, remove unecessary test setup and fix tests * [core]: update scripts + contributing scripts * [@svelteui/core]: Fixes #189 - fix Switch disabled style and add dark styles (#192) * [@svelteui/core]: fix switch disabled style and add dark mode style * [@svelteui/core]: fix switch disabled style and add dark mode style * [core]: migrate to new svelte-package approach since svelte-kit package is deprecated (#194) * [docs]: improve Modal target warning and demo (#197) * [@svelteui/core]: fix type inconsistency in actions API (#198) * [core]: move all packages outside lib folder (#196) * [@svelteui/preprocessors]: moved code from src/lib into src * [@svelteui/prism]: moved code from src/lib into src * [@svelteui/tests]: moved code from src/lib into src/ * [@svelteui/demos]: moved code from src/lib into src * [core]: removed static files from packages * [@svelteui/dates]: moved src/lib into src/ * [core]: fix packaging with new aliases: bump svelte-package with fix * [docs]: fix paths for components * [@svelteui/core]: fix test setup * [core]: fixed pub script and changed storybook to dev * [core]: made modal error more clear * [core]: [release] 0.7.3 * [core]: updated readme for rebase * [@svelteui/core]: fix Tab dark mode color and simple story for testing purposes (#201) * [@svelteui/core]: fix element retrieval in Modal (#203) * [@svelteui/core]: fix switch disable behaviour * [core]: bump svelte version * [core]: update yarn lock * [core]: do not change package.json on prettier since it changes the peerDependencies semver (#213) * [@svelteui/core]: add Chip and ChipGroup components (#209) * [@svelteui/core]: add Chip and ChipGroup components Stubbed first draft of components. Not ready yet. Mostly copied from other components like Checkbox and also Mantine. Not tested. Signed-off-by: Brian Evans <ebrian101@gmail.com> * [@svelteui/core]: change Chip styles after code review Edited the styles for Chip based on PR feedback. Used color functions where possible, instead of hard coding. Also changed the way variants are selected. Added a checkmark when a chip is selected. Signed-off-by: Brian Evans <ebrian101@gmail.com> * [@svelteui/core]: added Storybook story for Chip Signed-off-by: Brian Evans <ebrian101@gmail.com> * [@sveltui/core]: make Chip label dynamic Co-authored-by: Ana Margarida Silva <ams-amargarida@hotmail.com> * [@svelteui/core]: Chip styling and Story changes Based on PR feedback: - added more stories to storybook for sizing, and the different states - fixed dark mode selectors to use old method [`${theme.dark} &`] - made the label dynamic from label prop if no children provided to component Signed-off-by: Brian Evans <ebrian101@gmail.com> * [@svelteui/core]: hide input element in Chip Wrap the input element in a div which hides it. Signed-off-by: Brian Evans <ebrian101@gmail.com> * [@svelteui/core]: ChipGroup storybook Stories Added Stories for testing the ChipGroup. Removed some defaults from the ChipGroup component since these are set in Chip. Removed the input type prop, as it's not necessary. Signed-off-by: Brian Evans <ebrian101@gmail.com> * [@svelteui/core]: Chip disabled checked styling Signed-off-by: Brian Evans <ebrian101@gmail.com> Signed-off-by: Brian Evans <ebrian101@gmail.com> Co-authored-by: Kamell <71530812+Brisklemonade@users.noreply.github.com> Co-authored-by: Ana Margarida Silva <ams-amargarida@hotmail.com> * [docs]: fix docs paths (#214) * [docs]: fix docs paths for svelteui-core and edit page * [docs]: fix doc paths for composables * [docs]: fix doc paths for dates * [docs]: fix doc path for preprocessors * [@svelteui/core]: Fix menu placement bug (#216) The menu placement was not working when a custom control was used. I fixed the bug by making the custom control the reference, unless it doesn't exist, then use the default control element. The Story demonstrates the fix. Signed-off-by: Brian Evans <ebrian101@gmail.com> Signed-off-by: Brian Evans <ebrian101@gmail.com> * [core]: fix storybook global undefined error * [@svelteui/core]: fix type, TS errors and format * [docs] README documentation for NPM package (#218) * [docs]: Documentation for Chip and ChipGroup (#220) * [@svelteui/core]: fix Chip export * [@svelteui/core]: improvements to Chip and ChipGroup - support multiple or not * [@svelteui/demos]: chip demos * [@svelteui/core]: format fixes * [docs]: chip docs * [@svelteui/core]: use same path for linting command as in format * [core]: Fix npm dependency errors on install (#222) * [docs]: rename prepare script to not be called by husky * [core]: fix vite npm dependency error * [core]: add deno to contributing guide * [core]: small fix * [@svelteuidev/demos] Fixes #225: Gradient problem in docs (#228) * [@svelteui/core]: badge simple story and fix in prism styles * [docs]: add rollup to fix error on dev * [@svelteui/demos]: fix CompositeRender event dispatch * [@svelteui/core]: add IconRenderer (#223) * [core]: fixes #233 - update dev docs setup documentation * [@svelteui/core]: refactor import Signed-off-by: Brian Evans <ebrian101@gmail.com> Co-authored-by: Kamell Perry <kperry@ascend-innovations.com> Co-authored-by: Kamell <71530812+Brisklemonade@users.noreply.github.com> Co-authored-by: Brisklemonade <kamellperry33@gmail.com> Co-authored-by: Jason Xu <36783049+JasonXu314@users.noreply.github.com> Co-authored-by: Brian Evans <53117772+mrbrianevans@users.noreply.github.com> Co-authored-by: Kyle Shepherd <kyleshepherd13@gmail.com>
* [docs]: added missing installation check boxes and fixed typo * [core]: fixed typos * [core]: Yarn and Storybook migration (#160) * [migration]: all packages on kit-next.377 and use storybook template * [@svelteui/core]: configuring story book (viewport modifications and set up) * [@svelteui/core]: moved lib contents to src * [@svelteui/core]: fixed stories for button * [core]: fixed all eslintrcs * [core]: started theme setup for core package * [core]: storybook working with dark and light theme * [core]: revert some changes and remove logs * [core]: remove unused logic * [core] yarn migration (#157) * [core]: migrate to yarn * [docs]: update contribution guide to include yarn * [core]: modify github pipelines to use yarn * [core]: format and lint code * [core]: update yarn.lock (#161) Co-authored-by: BeeMargarida <ams-amargarida@hotmail.com> * [@svelteui/core]: support number as size in Button (#165) * [@svelteui/core]: fix missing prop type in NumberInput * [@svelteui/core]: use new prop approach in all components * [@svelteui/core]: remove onMouseEnter and onMouseLeave prop and use dispatch events instead * [@svelteui/core]: simplify prop approach in Button * [@svelteui/core]: format code - yarn format result * [@svelteui/core]: revert yarn format * [@svelteui/core]: remove BrowserRender, ServerRender and Fragment component (#167) * [core]: normalize scripts with : and udpdate readmes (#169) * [core]: sorted package jsons * [core]: updated docs and core dev scripts * [@svelteui/core]: Export all components prop types (#173) * [@svelteui/core]: export all component types instead of the whole styles * [core]: yarn lock * [core]: linting + format all package.json * [@svelteui/core]: fix select autocomplete logic that showed previous option on reload (#177) * [@svelteui/core]: fix Tabs bug where events inside tab content were not propagated due to node handling (#180) * [@svelteui/prism]: fix indentation problem on the first line (#181) * [@svelteui/prism]: fix indentation problem on the first line * [core]: remove package.json from prettier * [docs]: fix docs dev command, add docs node_modules to clean:all command and update tsconfig of core * [@svelteui/demos]: remove unecessary code from demos code examples (#182) * [@svelteui/core]: updated tsconfig * [@svelteuidev/core] Menu custom control (#184) * [@svelteui/core]: support for custom control in Menu * [@svelteui/core]: expose menu functions for opening/closing/toggling the menu state outside the component * [@svelteui/demos]: demo for Menu custom control * [docs]: add custom menu control examples to docs * [docs]: add missing period * [core]: remove flacky test * [@svelteui/core]: add role and aria info to custom menu control * [@svelteui/core]: added menu story * [core]: syncpack was failing script * [docs]: added modal clarification and removed banner * [core]: changed publish scripts due to unknown npm bug * [@svelteui/core]: menu story * [core]: fixed peerdeps versioning issues * [release]: 0.7.2 * [core]: fixed dev:core script * [@svelteui/composables]: migrated directory structure from the use of lib * [@svelteui/composables]: removed storybook config * [core]: fixed package json * [core]: changed yarn install to yarn in workflow * [core]: Storybook for core, composables and dates packages (#190) * [core]: removed unused stories from packages * [core]: moved storybook configuration to configuration folder and updated paths * [core]: working storybook that shows core stories * [core]: simple use-click-outside story for testing purposes * [core]: delete old storybook folders * [core]: stabilize storybook config * [@svelteui/composables]: change to * [@svelteui/dates]: change to * [core]: stories for dates, core and composables and simple example stories * [core]: format and lint fixes * [@svelteui/prism]: ignore line for prettier in prism * [@svelteui/composables]: small fix in alias * [@svelteui/composables]: small fixes to alias * [core]: bump vitest, cleanup dependencies, remove unecessary test setup and fix tests * [core]: update scripts + contributing scripts * [@svelteui/core]: Fixes #189 - fix Switch disabled style and add dark styles (#192) * [@svelteui/core]: fix switch disabled style and add dark mode style * [@svelteui/core]: fix switch disabled style and add dark mode style * [core]: migrate to new svelte-package approach since svelte-kit package is deprecated (#194) * [docs]: improve Modal target warning and demo (#197) * [@svelteui/core]: fix type inconsistency in actions API (#198) * [core]: move all packages outside lib folder (#196) * [@svelteui/preprocessors]: moved code from src/lib into src * [@svelteui/prism]: moved code from src/lib into src * [@svelteui/tests]: moved code from src/lib into src/ * [@svelteui/demos]: moved code from src/lib into src * [core]: removed static files from packages * [@svelteui/dates]: moved src/lib into src/ * [core]: fix packaging with new aliases: bump svelte-package with fix * [docs]: fix paths for components * [@svelteui/core]: fix test setup * [core]: fixed pub script and changed storybook to dev * [core]: made modal error more clear * [core]: [release] 0.7.3 * [core]: updated readme for rebase * [@svelteui/core]: fix Tab dark mode color and simple story for testing purposes (#201) * [@svelteui/core]: fix element retrieval in Modal (#203) * [@svelteui/core]: fix switch disable behaviour * [core]: bump svelte version * [core]: update yarn lock * [core]: do not change package.json on prettier since it changes the peerDependencies semver (#213) * [@svelteui/core]: add Chip and ChipGroup components (#209) * [@svelteui/core]: add Chip and ChipGroup components Stubbed first draft of components. Not ready yet. Mostly copied from other components like Checkbox and also Mantine. Not tested. Signed-off-by: Brian Evans <ebrian101@gmail.com> * [@svelteui/core]: change Chip styles after code review Edited the styles for Chip based on PR feedback. Used color functions where possible, instead of hard coding. Also changed the way variants are selected. Added a checkmark when a chip is selected. Signed-off-by: Brian Evans <ebrian101@gmail.com> * [@svelteui/core]: added Storybook story for Chip Signed-off-by: Brian Evans <ebrian101@gmail.com> * [@sveltui/core]: make Chip label dynamic Co-authored-by: Ana Margarida Silva <ams-amargarida@hotmail.com> * [@svelteui/core]: Chip styling and Story changes Based on PR feedback: - added more stories to storybook for sizing, and the different states - fixed dark mode selectors to use old method [`${theme.dark} &`] - made the label dynamic from label prop if no children provided to component Signed-off-by: Brian Evans <ebrian101@gmail.com> * [@svelteui/core]: hide input element in Chip Wrap the input element in a div which hides it. Signed-off-by: Brian Evans <ebrian101@gmail.com> * [@svelteui/core]: ChipGroup storybook Stories Added Stories for testing the ChipGroup. Removed some defaults from the ChipGroup component since these are set in Chip. Removed the input type prop, as it's not necessary. Signed-off-by: Brian Evans <ebrian101@gmail.com> * [@svelteui/core]: Chip disabled checked styling Signed-off-by: Brian Evans <ebrian101@gmail.com> Signed-off-by: Brian Evans <ebrian101@gmail.com> Co-authored-by: Kamell <71530812+Brisklemonade@users.noreply.github.com> Co-authored-by: Ana Margarida Silva <ams-amargarida@hotmail.com> * [docs]: fix docs paths (#214) * [docs]: fix docs paths for svelteui-core and edit page * [docs]: fix doc paths for composables * [docs]: fix doc paths for dates * [docs]: fix doc path for preprocessors * [@svelteui/core]: Fix menu placement bug (#216) The menu placement was not working when a custom control was used. I fixed the bug by making the custom control the reference, unless it doesn't exist, then use the default control element. The Story demonstrates the fix. Signed-off-by: Brian Evans <ebrian101@gmail.com> Signed-off-by: Brian Evans <ebrian101@gmail.com> * [core]: fix storybook global undefined error * [@svelteui/core]: fix type, TS errors and format * [docs] README documentation for NPM package (#218) * [docs]: Documentation for Chip and ChipGroup (#220) * [@svelteui/core]: fix Chip export * [@svelteui/core]: improvements to Chip and ChipGroup - support multiple or not * [@svelteui/demos]: chip demos * [@svelteui/core]: format fixes * [docs]: chip docs * [@svelteui/core]: use same path for linting command as in format * [core]: Fix npm dependency errors on install (#222) * [docs]: rename prepare script to not be called by husky * [core]: fix vite npm dependency error * [core]: add deno to contributing guide * [core]: small fix * [@svelteuidev/demos] Fixes #225: Gradient problem in docs (#228) * [@svelteui/core]: badge simple story and fix in prism styles * [docs]: add rollup to fix error on dev * [@svelteui/demos]: fix CompositeRender event dispatch * [@svelteui/core]: add IconRenderer (#223) * [core]: fixes #233 - update dev docs setup documentation * [@svelteui/core]: refactor import * [core]: update tsconfigs * [@svelteui/core]: make Code props optional (#238) * [@svelteuidev/core] Fix modal target problem (#239) * [@svelteui/core]: fix modal target check and affix props * [@svelteui/demos]: fix modal demos * [@svelteui/demos]: revert target=body in modal demos * [docs]: Changed the line-height and added animations on scroll in the homepage (#240) * [docs]: Changed lineHight of text in homepage to match Features.svelte * [docs]: Added transitions on scroll for Features.svelte * [docs]: Changed variable name to improve clarity * [docs]: Ran tests and linted code * [docs]: Changed height of wrapper div * [docs]: fix height in docs * [core]: update README * [docs]: Improvements in sentences and changed styling of NextSteps component (#244) * [docs]: Changed lineHight of text in homepage to match Features.svelte * [docs]: Added transitions on scroll for Features.svelte * [docs]: Changed variable name to improve clarity * [docs]: Ran tests and linted code * [docs]: Changed height of wrapper div * [docs]: Improvements in sentences and styling * [docs]: Hopefully resolved conflict * review changes * [@svelteui/demos] Fixed incorrect value being shown (#247) * [core]: exclude peer deps in syncpack set-semver-ranges script (#246) * [@svelteui/core]: fix Grid spacing prop reactivity and style (#248) * [@svelteui/core]: exclude open/close events from event forwarding in Meny (#245) * [@svelteuidev/composables] Fixes #241: focus trap action and usage on modal (#243) * [@svelteui/composables]: use-focus-trap action * [@svelteui/core]: use focus-trap action in modal * [@svelteui/composables]: story for action focus-trap * [docs]: focus-trap docs * [@svelteuidev/composables] use-move action (#242) * [@svelteui/composables]: use-move action to move an element with the mouse movement * [@svelteui/composables]: set custom events for native DOM elements typings * [@svelteui/composables]: use-move finished with working story * [@svelteui/composables]: export use-move * [@svelteui/composables]: fix namespace and story style * [@svelteui/demos]: use-move demo * [docs]: use-move docs * [docs]: fix in description * Modal story added Signed-off-by: Brian Evans <ebrian101@gmail.com> Co-authored-by: Kamell Perry <kperry@ascend-innovations.com> Co-authored-by: Kamell <71530812+Brisklemonade@users.noreply.github.com> Co-authored-by: Brisklemonade <kamellperry33@gmail.com> Co-authored-by: Jason Xu <36783049+JasonXu314@users.noreply.github.com> Co-authored-by: Brian Evans <53117772+mrbrianevans@users.noreply.github.com> Co-authored-by: Kyle Shepherd <kyleshepherd13@gmail.com> Co-authored-by: Caladan <caladanofdune@gmail.com>
The goal of this MR is to fill a gap of a piece of functionality that seems to be missing.
Currently in SvelteUI, multiple components that take a
icon
prop accept this as either aComponent
(aSvelteComponentDev
type wrapper) or as aHTMLOrSVGElement
. However, if a variable of typeHTMLOrSVGElement
is passed to one of these components, an error is returned as the icon is rendered using<svelte:component/>
.This MR adds a wrapper component called
IconRenderer
that is used in these various components and will check the type of theicon
prop and then render it accordingly.Before submitting the PR, please make sure you do the following
[@svelteui/core]
,[@svelteui/actions]
,[@svelteui/motion]
,[@svelteui/core]
,[core]
, or[docs]
.Tests
npm test
and lint the project withyarn lint
or just runyarn repo:prepush
and check to see if it's passing.