-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Feature/#551 Expose a metric control #1272
Feature/#551 Expose a metric control #1272
Conversation
Adding pytest cases for Metric.tsx components
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
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.
A very good first PR
Needs a few changes but everything is there
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
---|---|---|---|
🟢 | Statements | 81.17% (+0.14% 🔼) |
2587/3187 |
🟡 | Branches | 62.62% (-0.68% 🔻) |
1642/2622 |
🟡 | Functions | 74.21% (+0.06% 🔼) |
446/601 |
🟢 | Lines | 81.74% (+0.12% 🔼) |
2413/2952 |
Show new covered files 🐣
St.❔ |
File | Statements | Branches | Functions | Lines |
---|---|---|---|---|---|
🟢 | ... / Dialog.tsx |
80.43% | 60.87% | 100% | 80.43% |
🟡 | ... / TaipyRendered.tsx |
64.58% | 14.29% | 50% | 64.58% |
🟡 | ... / index.ts |
75% | 26.67% | 50% | 74.47% |
🟢 | ... / Expandable.tsx |
100% | 85% | 100% | 100% |
🟢 | ... / Metric.tsx |
100% | 68.18% | 100% | 100% |
🟢 | ... / PageContent.tsx |
75% | 100% | 0% | 100% |
🟢 | ... / Pane.tsx |
95.12% | 73.81% | 100% | 94.87% |
🟢 | ... / Part.tsx |
88% | 64.29% | 75% | 91.3% |
🟡 | ... / Unregistered.tsx |
62.5% | 0% | 0% | 60% |
Test suite run success
368 tests passing in 39 suites.
Report generated by 🧪jest coverage report action from af4dd42
I love the description you provided 😉 . Thx! |
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.
another few comments
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.
Almost there
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.
Very minor comments.
Well done!
…iga/taipy into feature/#551-Exposing-metric-control
…iga/taipy into feature/#551-Exposing-metric-control
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.
Well done
Can you create another issue to update the stylekit css?
https://vscode.dev/github/Avaiga/taipy/blob/feature/%231037-date-format/frontend/taipy-gui/public/stylekit/controls/chart.css#L24
And we also need to support hover_text
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.
Minor readability comments.
type: "indicator", | ||
mode: "gauge" + (showValue ? "+number" : "") + (delta !== undefined ? "+delta" : ""), | ||
delta: { | ||
reference: typeof value === 'number' && typeof delta === 'number' ? value - delta : undefined, |
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.
For better readability, I would change that to
(value === undefined || delta === undefined) ? undefined : (value - delta)
that is more efficient and more readable (value and delta ARE numbers - except when they're not).
gauge: { | ||
axis: { | ||
range: [ | ||
typeof props.min === 'number' ? props.min : 0, |
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.
Same comment here.
The exception is 'undefined' - not (not number).
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.
we're never sure of what we receive, I think it is safer this way.
551
Introduces the "metric" element to the project, facilitating synthetic data representation with customizable controls. Key properties include:
This addition enhances data visualization capabilities, providing adaptability and precision.