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

[NcAppSettingsDialog] Allow to add icons to the navigation sections #4745

Merged
merged 2 commits into from Nov 7, 2023

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Nov 2, 2023

☑️ Resolves

This consists of 2 commits, the first one changes the way the sections are registered and the second on provides icon support.

Section registration

Previously the default slot content was iterated and the props of every vNode were used. This had two downsides:

  1. $slots is not reactive -> changes will not propagate
  2. We can not easily access slots of a section (like for the icon) because the slot will only be available when the vNode is mounted

So instead we now provide to functions to children:
registerSection and unregisterSection with that functions children (the sections) can register themself and update on change. Solving both issues: the reactivity and the ability to inject icons (as the registration is done after mount).

I personally also think it is easier to understand as no longer internals of vNodes need to be handled, but this is personal taste.

Icon support

There is a new slot icon on the settings section which allows to inject icons to the section name in the navigation.

🖼️ Screenshots

With this changes it loos like this:

without icons (looks like before) with icons
Screenshot_20231102_021445 Screenshot_20231102_021453

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

Instead of reading the default slot on mount time we use injected `(un)registerSection` functions.
Those can be called by the section to register themself or unregister.
This makes the sections reactive and requires less work with internal vnode data.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
…e in the navigation

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux added enhancement New feature or request 3. to review Waiting for reviews feature: settings Related to the settings component labels Nov 2, 2023
@susnux susnux merged commit 9fb90b7 into master Nov 7, 2023
15 checks passed
@susnux susnux deleted the feat/app-settings-icons branch November 7, 2023 10:32
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: settings Related to the settings component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants