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.

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: inconsistent default slot naming in accordian item #5437

Closed
dirthsj opened this issue Dec 4, 2021 · 7 comments · Fixed by #5514
Closed

fix: inconsistent default slot naming in accordian item #5437

dirthsj opened this issue Dec 4, 2021 · 7 comments · Fixed by #5514
Labels
bug A bug community:good-first-issue Good issues for first time contributors community:request Issues specifically reported by a member of the community. docs:spec A technical specification status:needs-investigation Needs additional investigation

Comments

@dirthsj
Copy link

dirthsj commented Dec 4, 2021

🐛 Bug Report

fast-accordion's definition available here has a name for the default slot; elsewhere the default slot is named empty string.

💻 Repro or Code Sample

No code is required. Read the component definition(s) on the component explorer and compare the default slot names.

🤔 Expected Behavior

The default slot should be named empty string

😯 Current Behavior

The default slot is named item

💁 Possible Solution

Rename slot to empty string

🔦 Context

I was attempting to generate svelte components with typescript definitions from the JSON definition(s) of the components. I noticed this inconsistency when it caused my generated code to not work as expected until I removed the name from this slot in my generated svelte component.

🌍 Your Environment

N/A

@dirthsj dirthsj added the status:triage New Issue - needs triage label Dec 4, 2021
@EisenbergEffect EisenbergEffect added area:fast-components bug A bug community:request Issues specifically reported by a member of the community. docs:spec A technical specification status:needs-investigation Needs additional investigation and removed status:triage New Issue - needs triage labels Dec 6, 2021
@EisenbergEffect
Copy link
Contributor

@chrisdholt Not sure if this is an error in the spec, a bug in the component, or something wrong in the documentation or definition file.

@eljefe223
Copy link
Contributor

@EisenbergEffect it appears the accordion template has a name on the default slot. We believe we should just remove the name

@chrisdholt
Copy link
Member

I think the issue here is that removing this will break the current templates for anyone who is projecting using name. If we remove those will no longer project.

My proposal here would be to add a generic/default slot in addition to the named slot and then track the removal of that with an issue AND a TODO in the codebase for the next major version.

@chrisdholt chrisdholt added the community:good-first-issue Good issues for first time contributors label Dec 10, 2021
@chrisdholt
Copy link
Member

Marking as good first issue for anyone in the community who wants to pick this up.

@m4thieulavoie
Copy link
Collaborator

@chrisdholt I'm happy to contribute a PR!

@chrisdholt
Copy link
Member

Thanks!

@chrisdholt
Copy link
Member

@m4thieulavoie pinging here because I don’t recall your @ on discord. Can you shoot me a message there when you get a second. Thanks again :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug community:good-first-issue Good issues for first time contributors community:request Issues specifically reported by a member of the community. docs:spec A technical specification status:needs-investigation Needs additional investigation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants