kirbydesign / designsystem

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

[Enhancement] Input with affix adjustment #2573

Open henrikvoetmand opened 1 year ago

henrikvoetmand commented 1 year ago

Describe the enhancement

Adjustment to input with affix. We want to align Icon and icon-btn affix.

Describe the solution you'd like

Adjust spacing and placement of icon and icon btn. Consider limiting the affix elements to only icons, icon buttons and text.

Have you considered any alternatives?

Are there any additional context?

https://zpl.io/Z08z7Kd

Input-affix-icon-btn


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:

tspringborg commented 1 year ago

A few thoughts regarding this issue. The current functionality allows for everything to be inserted. It just needs a kirby-affix directive which is valid on any element. Elements are centered vertically with flex image

So any instances where an element seems to not be vertically aligned is the fault of the inserted element itself. Which can be fixed with individual styling on the inserted element. (In your application) Personally I don't think it would be a good idea to start implementing styling logic that is conditional on the affix element type.

In regards to horizontal spacing it is derived from default kirby-input horizontal padding from affix to the relevant edge (suffix = padding-right / prefix = padding-left) while the "inside" padding (between text and affix) is half of default kirby-input padding. These padding values are easy to change and if we can solve most issues that way, I think it would be the best solution.

When it comes to limiting affix element to icons, icon buttons and text I am strongly apposed to this. Some reasons why I don't like it:

  1. We add more complexity to the kirby code base while gaining very little (imo)
  2. We decrease the usefulness of kirby form field affix functionality. (by limiting what can be inserted, and quite possibly, depending on the implementation and the inserted element, also how it can be styled)
  3. We increase the maintenance-factor by requiring the kirby team to keep the "valid-insertable-affix-elements" list of components and elements updated.
  4. We increase the maintenance/complexity factor by requiring the kirby team to consider any styling changes to kirby-components present on the "valid-insertable-affix-elements" list in both normal context and affix context.

I hope this makes sense :)

TLDR:

tspringborg commented 1 year ago

I believe @henrikvoetmand and I came to this conclusion. Affix-spec

So 16/12px fixed padding.

If this is acceptable. It should be easy to move forward.