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(NcAppSidebar): restore attrs and classes inheritance #5688

Closed
wants to merge 1 commit into from

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Jun 11, 2024

☑️ Resolves

Makes <NcAppSIdebar> with Fragment work like there is no Fragment

This PR restores it in quite a dirty way. I don't see any better solution without a breaking change.

Example of broken apps - Files:

  • No cy-data-sidebar attributes for e2e
  • No app-sidebar--full class
  • No data-v-{scopeId} attr required for app-sidebar--full

🖼️ Screenshots

🏚️ Before 🏡 After
image image

🚧 Tasks

  • Pass attrs and listeners on <aside> directly
  • Manually take and bind classes and styles from VNode
  • Manually set data-v-{scopeid} from $parent

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 3️⃣ Backport to next requested with a Vue 3 upgrade

@ShGKme ShGKme added bug Something isn't working 2. developing Work in progress regression Regression of a previous working feature feature: app-sidebar Related to the app-sidebar component labels Jun 11, 2024
@ShGKme ShGKme added this to the 8.13.0 milestone Jun 11, 2024
@ShGKme ShGKme self-assigned this Jun 11, 2024
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

This one is quite nice, small and works 👍

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme
Copy link
Contributor Author

ShGKme commented Jun 19, 2024

Closing in flavor of #5627

@ShGKme ShGKme closed this Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working feature: app-sidebar Related to the app-sidebar component regression Regression of a previous working feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants