kirbydesign / designsystem

Kirby Design System
https://cookbook.kirby.design
MIT License
85 stars 23 forks source link

[Enhancement] Clarify strategy for kirbyfying local-nav component #2619

Closed mictro closed 1 year ago

mictro commented 1 year ago

Describe the enhancement

A meeting was held with the Kirby team on Tuesday the 25th of November in order to scope the task of kirbyfication of the experimental component local-navigation built by @RasmusTraeholt for the investment team. It was decided: (see below)

Describe the solution you'd like

In scope: o Move the component to the kirby component library and give it a new name. o Change the component to be 'more' 'declarative' but exactly how 'declarative' is to be clarified. o Document the new component in the cookbook

Out of scope: o Make the component ready for desktop will be done later (together with the remaining Kirby components). o Extending the current functionality of the local-nav component. o Merging the current kirby tab component into a single tab component.

Clarifications: The component should be made more 'declarative' but exactly how is to be clarified - see below. The the name of the component has to be decided. The component should be documented together with the kirby-page or kirby-tabs cookbook entry.

Have you considered any alternatives?

Are there any additional context?

An example of the local-navigation with kirby-page can be seen at: https://cookbook.kirby.design/#/examples/experimental/local-navigation


Checklist:

The following tasks should be carried out in sequence in order to follow the process of contributing correctly.

Refinement

Implementation

The contributor who wants to implement this issue should:

Review

Once the issue has been implemented and is ready for review:

mictro commented 1 year ago

Clarification on the how of making the component more 'declarative'

Below are sketched three different options inspired by the current structure of similar components in the Kirby component library - there may be other options as well. The weighting of pros/cons are neither succinct nor objective and depends heavily on what is to be achieved.

mictro commented 1 year ago

Option A: 'Kirby Segmented Control' Flavor

navItems: TabNavigationItem[] = [
  {
    text: ...
    badge: {
      content: ..,
      icon: ..., 
      description: ...,
      themeColor: ... 
  },
]  
<kirby-tab-navigation [navItems]=tabItems (itemSelected)="onItemSelected($event)" ...>
</kirby-tab-navigation>

Option A: + Closed component mark-up structure - better encapsulation. + Easier to make changes to component due to known usage scenarios - better encapsulation. + More programmatic approach to use the component - programmers might like that. + Better integrity and robustness of the component - not that easy to make it break. + Easier to add usefull logic to the component without probing for mark-up - structure is known. + Easier and quicker to implement - less dependency on supplied components and not restricted to existing kirby components.

- Only possible to use the component for it's intended purpose - which might also be a good thing. - Component is difficult to change without involving Kirby. - Provided logic and component segmentation might not fill the need.
- Not easy for the programmer to make adjustments to the component - might also be a good thing.

mictro commented 1 year ago

Option B: 'Kirby Segmented Control' Unconvoluted Flavor

<kirby-tab-navigation>
  <kirby-tab-navigation-button [text]=... [badge]=... (click)="onButtonClicked($event)">
  </kirby-tab-navigation-button>
  <kirby-tab-navigation-button ...>
  </kirby-tab-navigation-button>
  ...
</kirby-tab-navigation>

Option B: +   Better mark-up aesthetics than A. +- Mostly resembles option A with a little move towards C. +- Delegates selection handling to component client - not handled internally as in A. +- Still not cutomizable by the programmer as in A - which may also be a good thing.

mictro commented 1 year ago

Option C: 'Kirby Tabs' Flavor

<kirby-tab-navigation>
  <kirby-tab-navigation-button>
     ...
  </kirby-tab-navigation-button>
  <kirby-tab-navigation-button>
     ...
  </kirby-tab-navigation-button>
  ...
</kirby-tab-navigation>

Example:

<kirby-tab-navigation>
  <kirby-tab-navigation-button (click)="onButtonClicked($event)">
    Inbox
    <kirby-badge themeColor="danger">1</kirby-badge>
  </kirby-tab-navigation-button>
  <kirby-tab-navigation-button>
    Attachments
    <kirby-badge>
      <kirby-icon name="attach"></kirby-icon>
    </kirby-badge>
  </kirby-tab-navigation-button>
...
</kirby-tab-navigation>

Option C: + Open component mark-up structure - delegate part of the construction to the programmer. + Component can be made usable in other usage scenarios than it originally was designed for - legally hackable. + Appealing mark-up aesthetics - Visibly builds upon existing Kirby components - Lego Principle. + Only a small amount of logic in the component - mostly visual styling.

- Only a few supported mark-up patterns. - Customizable by developers but at their own risk. - Small deviations from documented mark-up patterns might break the design. - Missing support for undocumented usage patterns - may be difficult for developers to grasp. - Customization requires disciplined and skilled developers - will Kirby be responsible for supporting? - May be error-prone/impossible to make changes to the component due to the wide range of usage scenarios - lack of encapsulation and control.

RasmusKjeldgaard commented 1 year ago

Great write-up 👍

To be honest, I am all for option C, and is is also slowly the direction Kirby has been moving lately. With so many teams using us, we really need the composability, so every request for a simple feature (imagine a badge to the left, instead of to the right) does not have to go through us.

As we have talked about previously, we can mitigate a lot of the need for 'hacking' with sensible defaults that Just Work™.

I think it is a strength that developers can put whatever they need inside the tabs, and I do not think this makes it that much harder to maintain the component because it is still its own encapsulated entity.

I guess click events from the tab-navigation-button should bubble up to tab-navigation so we can do the selection handling/scrolling the button into view from there. Worth PoC'ing I guess 😄