-
Notifications
You must be signed in to change notification settings - Fork 18
Update partition metric template to follow nova 5 template #62
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
base: master
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.
Pull Request Overview
This PR updates the BasePartitionMetric component template to align with Nova 5's original template design, reverting custom modifications that were causing layout inconsistencies when using this package.
- Adds
legends-heightprop to control legend container height behavior - Updates chart positioning from absolute to flexbox layout
- Reverts chart styling to match Nova 5's original implementation
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| LinkablePartitionMetric.vue | Passes new legends-height prop to base component |
| BasePartitionMetric.vue | Major template restructure to match Nova 5 original with flexbox layout and conditional height controls |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| :class="chartClasses" | ||
| style="width: 90px; height: 90px; bottom: 30px; top: calc(50% + 15px)" | ||
| class="flex-none rounded-b-lg ct-chart mr-4 w-[90px] h-[90px]" | ||
| :class="{ invisible: this.currentTotal <= 0 }" |
Copilot
AI
Oct 9, 2025
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.
Remove 'this.' prefix when referencing computed properties or data in templates. Vue templates automatically bind to the component context.
| :class="{ invisible: this.currentTotal <= 0 }" | |
| :class="{ invisible: currentTotal <= 0 }" |
| this.chartist = new Chartist.Pie(this.$refs.chart, this.formattedChartData, { | ||
| donut: true, | ||
| donutWidth: 10, |
Copilot
AI
Oct 9, 2025
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.
The donutWidth variable is declared but then a hardcoded value of 10 is used in the configuration object. Either use the variable consistently or remove the unused variable declaration.
| donutWidth: 10, | |
| donutWidth: donutWidth, |
The BasePartitionMetric.vue is different then the one shipped from nova itself. So when using this package, the layout is changed. Revert template back to original and let users overwrite when needed.