Skip to content
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.

Sign up for GitHub

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

Fix Tab-Group Header not displayed #2557

Merged
merged 11 commits into from
Jul 13, 2024
Merged

Fix Tab-Group Header not displayed #2557

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ab7984b
Add content to named slot
gerteck Jun 13, 2024
624fa5b
Remove header prop
gerteck Jun 13, 2024
f92b856
Add props for TabGroup header
gerteck Jun 20, 2024
c213c5c
Update tabgroup header prop name
gerteck Jul 1, 2024
b1a0055
Update tab group header prop
gerteck Jul 1, 2024
a5c6934
Add slot for header back
gerteck Jul 4, 2024
1dd2449
Rename prop name
gerteck Jul 4, 2024
2168083
Add Tabset, Tab, Tabgroup snapshot test
gerteck Jul 4, 2024
0cdf3d0
Merge branch 'master' into branch-TabGroupHeaderFix
gerteck Jul 4, 2024
a5da4c4
Refactor import
gerteck Jul 4, 2024
49ab2db
Merge branch 'branch-TabGroupHeaderFix' of https://github.com/gerteck…
gerteck Jul 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/vue-components/src/Dropdown.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
:class="{'disabled': disabledBool}"
data-bs-toggle="dropdown"
>
<slot name="header"></slot>
<span v-html="tabgroupHeader"></span>

Check warning on line 14 in packages/vue-components/src/Dropdown.vue

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest) 

'v-html' directive can lead to XSS attack

Check warning on line 14 in packages/vue-components/src/Dropdown.vue

View workflow job for this annotation

GitHub Actions / test (macos-latest) 

'v-html' directive can lead to XSS attack

Check warning on line 14 in packages/vue-components/src/Dropdown.vue

View workflow job for this annotation

GitHub Actions / test (windows-latest) 

'v-html' directive can lead to XSS attack
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I'm not keen on this naming since this is only true for tab groups but not for other headers.
Could we keep the header name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! The rationale was give greater differentiation in the way the header is passed as a prop instead of relying on slot conversion, but it is evident from the code directly.

I would be down to try writing some test cases for the tabs! Test cases would also prevent any further regressions until a better way to pass down the header is found.

Copy link
Contributor Author

@gerteck gerteck Jul 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I just realised that setting it as the same name header causes two of the current tests to fail under dropdown as the header no longer shows up as an attribute in the html rendered.

This is probably because of the slot name and the attribute prop name having the same name.

With that in mind, I think that the tab-group-header prop name should be differentiated from the header slot name as it can get confusing as well.

</a>
</slot>
<slot name="dropdown-menu" :class="[{ 'show': show }, { 'dropdown-menu-end': menuAlignRight }]">
Expand Down Expand Up @@ -70,6 +70,10 @@
type: String,
default: 'light',
},
tabgroupHeader: {
type: String,
default: '',
},
menuAlignRight: {
type: Boolean,
default: false,
Expand Down
2 changes: 1 addition & 1 deletion packages/vue-components/src/Tabset.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
v-else
:key="index"
class="nav-item"
:header="t.headerRendered"
:class="{active:t.active}"
:tabgroup-header="t.headerRendered"
:disabled="t.disabled"
>
<li v-for="(tab, tabIndex) in t.tabs" :key="tabIndex">
Expand Down
Loading