Skip to content

Conversation

@Antreesy
Copy link
Contributor

@Antreesy Antreesy commented Mar 28, 2025

☑️ Resolves

  • Allows to put custom content with info slot
  • Expose var(--app-sidebar-close-button-offset) for custom content positioning (respecting the space taken by close button)
  • Introduce NcAppSidebarHeader for a11y navigation

See https://github.com/nextcloud-libraries/nextcloud-vue/pull/6666/files?w=1 for non-whitespace changes
See https://github.com/nextcloud-libraries/nextcloud-vue/tree/feat/noid/appsidebar-header-empty-slot-vue2 for Vue2 branch to test with apps

🖼️ Screenshots

Quick prototype for Talk:

Clean WIth explanation
image image
2025-04-23_13h59_29.mp4

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 3️⃣ Backport to stable8 requested

@Antreesy Antreesy added enhancement New feature or request 3. to review Waiting for reviews feature: app-sidebar Related to the app-sidebar component labels Mar 28, 2025
@Antreesy Antreesy added this to the 8.24.0 milestone Mar 28, 2025
@Antreesy Antreesy self-assigned this Mar 28, 2025
@Antreesy Antreesy force-pushed the feat/noid/appsidebar-header-empty-slot branch 2 times, most recently from 54cd97c to 9d6aba4 Compare March 31, 2025 07:50
@Antreesy Antreesy changed the title feat(NcAppSidebar): provide 'header-empty' slot for customization feat(NcAppSidebar): add 'content' slot and 'noClose' prop Mar 31, 2025
@Antreesy Antreesy mentioned this pull request Mar 31, 2025
@Antreesy Antreesy force-pushed the feat/noid/appsidebar-header-empty-slot branch from 9d6aba4 to f086d2e Compare March 31, 2025 09:06
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.

technically makes sense - not sure about design.
But in general I like the idea of splitting it into a layout component and the specialized components, so 👍

@Antreesy Antreesy requested a review from ShGKme March 31, 2025 17:56
Copy link
Contributor

@DorraJaouad DorraJaouad left a comment

Choose a reason for hiding this comment

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

Makes sense

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.

Paddings seem to be off, also close button will not be placed consistent.
I think we need to polish the implementation a bit before pushing this to public API.

@ShGKme ShGKme modified the milestones: 8.24.0, 8.25.0 Apr 2, 2025
@Antreesy Antreesy force-pushed the feat/noid/appsidebar-header-empty-slot branch from ce12f05 to 8319b4e Compare April 8, 2025 13:27
@Antreesy Antreesy changed the base branch from stable8 to main April 8, 2025 13:27
@Antreesy
Copy link
Contributor Author

Antreesy commented Apr 8, 2025

Added --app-sidebar-close-button-offset variable exposed to <header class="app-sidebar-header"> descendants:

noClose - true noClose - false
image image

The rest of paddings are set by default header content, and should not be enforced to custom content (like with background-image, there might be a need to fill all the space available)

As all of the new stuff, moved to main branch to settle it here first.

@Antreesy Antreesy modified the milestones: 8.25.0, v9.0.0-rc.0 Apr 8, 2025
@Antreesy Antreesy requested a review from susnux April 8, 2025 13:37
@susnux susnux added this to the 9.0.0 milestone Apr 16, 2025
@Antreesy Antreesy force-pushed the feat/noid/appsidebar-header-empty-slot branch from 8319b4e to d6e9b39 Compare April 24, 2025 21:35
@Antreesy Antreesy changed the title feat(NcAppSidebar): add 'content' slot and 'noClose' prop feat(NcAppSidebar): add 'content' slot Apr 24, 2025
@Antreesy Antreesy force-pushed the feat/noid/appsidebar-header-empty-slot branch from b22c2e1 to 070eb52 Compare April 25, 2025 09:45
@Antreesy
Copy link
Contributor Author

/backport! to stable8

@ShGKme
Copy link
Contributor

ShGKme commented Apr 25, 2025

Proposals:

  • Rename a new slot to #info or something else to not mix with the entire sidebar content
  • Hide new slot in empty state
  • Add a hidden header for empty state
  • For research - check if we need to replace __info block or define custom __info content
  • Follow-up: fallback <NcAppSidebarHeader> name and title to <NcAppSidebar>'s corresponding values

- var(--app-sidebar-close-button-offset)

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy Antreesy force-pushed the feat/noid/appsidebar-header-empty-slot branch from 953262f to b94acdf Compare April 25, 2025 11:18
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy Antreesy force-pushed the feat/noid/appsidebar-header-empty-slot branch from b94acdf to e818f86 Compare April 25, 2025 11:43
Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

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

I like it

image

@ShGKme ShGKme changed the title feat(NcAppSidebar): add 'content' slot feat(NcAppSidebar): add info slot Apr 25, 2025
@ShGKme ShGKme requested a review from susnux April 25, 2025 11:52
@Antreesy Antreesy merged commit a4b2100 into main Apr 25, 2025
29 checks passed
@Antreesy Antreesy deleted the feat/noid/appsidebar-header-empty-slot branch April 25, 2025 12:51
@susnux susnux mentioned this pull request May 12, 2025
4 tasks
@Antreesy Antreesy modified the milestones: 9.0.0, 9.0.0-rc.1 Sep 11, 2025
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 enhancement New feature or request feature: app-sidebar Related to the app-sidebar component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants