Skip to content

✨ [progress] add progress#10

Open
Muzych wants to merge 2 commits intoshuimo-design:mainfrom
Muzych:main
Open

✨ [progress] add progress#10
Muzych wants to merge 2 commits intoshuimo-design:mainfrom
Muzych:main

Conversation

@Muzych
Copy link
Copy Markdown
Contributor

@Muzych Muzych commented Mar 15, 2024

🤔 Nature of this PR

  • 🐛 bug fix
  • ✨ new feature
  • 📝 documentation improvement
  • 🛝 demo code improvement
  • 🚀 component style/interaction improvement
  • 🏗️ ci/cd improvement
  • ♻️ refactoring
  • 🎨 code style optimization
  • ✅ test cases
  • 🔀 branch merging
  • 💡 other

🔗 Related Issue

💡 Background and Solution

✅ Pre-merge Checklist

❗️Please self-check and check all options.❗️

  • Documentation is supplemented or not needed
  • Code demonstration is provided or not needed
  • TypeScript definitions are supplemented or not needed
  • Changelog is provided or not needed

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

defineCore in vue version has some "historical reasons".
I don't think this is necessary in react version.

export const leaf = leafPng;


export function useProgress(options: Options<{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

try this:

export function useProgress(props:ProgressProps) 

@@ -0,0 +1,65 @@
/**
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

copy file from vue version , you should update comments

useEffect(() => {
setValue(props.value ?? 0);
setMax(props.max ?? 100);
}, [props.value, props.max]);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should value and max use two useEffect?
Their changes don't seem to affect each other.

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