[Component]: Breadcrumb Component (Work in progress) #861
Conversation
❌ Deploy Preview for oruga-documentation-preview failed.
|
mouadTaoussi
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #861 +/- ##
============================================
- Coverage 56.71% 17.61% -39.11%
============================================
Files 30 301 +271
Lines 1511 7420 +5909
Branches 544 2126 +1582
============================================
+ Hits 857 1307 +450
- Misses 654 5160 +4506
- Partials 0 953 +953
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| }, | ||
| align: { | ||
| type: String, | ||
| default: getOption("breadcrumb.size"), |
| /** | ||
| * This is used internally | ||
| * @ignore | ||
| */ |
|
|
||
| // --- Computed Component Classes --- | ||
| const computedTag = computed(() => { | ||
| return props.tag ? props.tag : "ul"; |
There was a problem hiding this comment.
Why not provide a default?
You can do default: () => getOption("breadcrumb.tag", "ul"),
| "separatorClass", | ||
| "o-breadcrumb--", | ||
| computed(() => props.separator), | ||
| computed(() => !!props.separator), |
There was a problem hiding this comment.
What is the separator class for? I would think of a class on the separator element, but not on the root with a suffix.
| }, | ||
| tag: { | ||
| type: String, | ||
| default: () => getOption("breadcrumb.tag"), |
There was a problem hiding this comment.
Please define the tag as similar to other components:
tag: {
type: [String, Object, Function] as PropType,
default: () => getOption("breadcrumb.tag", "li"),
},
| // }, | ||
| disabled: { | ||
| type: String, | ||
| default: () => getOption("breadcrumb.disabled"), |
| :icon="iconLeft" | ||
| :both="iconBoth" | ||
| :class="[...iconClasses, ...iconLeftClasses]" /> | ||
| <slot></slot> |
There was a problem hiding this comment.
I would consider the sperator to be some html element.
| <template> | ||
| <component :is="computedTag" :class="rootClasses" data-oruga="breadcrumb"> | ||
| <!-- BreadcrumbItems --> | ||
| <slot></slot> |
There was a problem hiding this comment.
I would like to use the component like
const route = useRoute();
<o-breadcrumb :route="route" />
And the component would render the current route with its breadcumb items.
|
Feel free to reopen it when you have further development to show. I would also prefer that you open a PR on your fork first, if you want me to have a look. Otherwise it would be cleaner to reopen this when it is ready. |
|
|
||
| import { registerComponent } from "@/utils/plugins"; | ||
|
|
||
| /** export button plugin */ |
| override: { type: Boolean, default: undefined }, | ||
| /** | ||
| * Color variant of the breadcrumb | ||
| * @values primary, info, success, warning, danger, and any other custom color |
| * Icon pack to use | ||
| * @values mdi, fa, fas and any other custom icon pack | ||
| */ | ||
|
|
There was a problem hiding this comment.
fomatting - unnecessary empty lines
| type: [String, Array, Function] as PropType<ComponentClass>, | ||
| default: undefined, | ||
| }, | ||
| iconBoth: { type: Boolean, default: false }, |
There was a problem hiding this comment.
missing js doc and its not a class prop -> move more up
| }); | ||
|
|
||
| // --- Computed Component Classes --- | ||
| const computedTag = computed(() => { |
There was a problem hiding this comment.
Why having a prop default and a computed? computed is not necessary
package.json
Outdated
| @@ -34,5 +34,8 @@ | |||
| "conventional-changelog": "^5.1.0", | |||
| "conventional-changelog-cli": "^4.1.0", | |||
| "replace-in-file": "^7.1.0" | |||
| }, | |||
| "dependencies": { | |||
| "jest-axe": "^8.0.0" | |||
Fixes #862
Proposed Changes