feat(core): replace TUI_DROPDOWN_OFFSET with injection token

[bondarvladislave]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows Conventional Commits
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Refactoring
• Code style update
• Build or CI related changes
• Documentation content changes

What is the current behavior?

In different design systems, the tui-data-list may contain paddings other than 4px, which are set by the TUI_DROPDOWN_OFFSET constant. I would like to do this as an injection token so that the dropdown does not cover the content.

What is the new behavior?

Does this PR introduce a breaking change?

• Yes
• No

[lumberjack-bot]

Pull request was closed ✔️

All saved screenshots (for current PR) were deleted 🗑️

[waterplea]

How about we add it to options? And also maybe it makes sense to make it two values: horizontal and vertical?

[bondarvladislave]

How about we add it to options? And also maybe it makes sense to make it two values: horizontal and vertical?

@waterplea

Yes, it really doesn't make sense to me if we don't add vertical and horizontal ones.

If added to options then we need to extend TuiDropdownOptionsDirective with selectors tuiDropdownHorizontalOffset and tuiDropdownVerticalOffset ?

But I would consider adding this to all places in the api documentation.
Because horizontal offset will work as expected only in tuiDropdownSided.

And for a regular tuiDropdown, it is difficult to describe in the api example why nothing happens until a specific number. (on screenshot)
I don’t know how to properly separate the logic so as not to confuse users.

And maybe you remember why you created [tuiDropdownCustomPosition]?
Maybe this was the beginning for some future solution on this topic

[waterplea]

Hmmm. True. Then it doesn't make sense to add both, if only one would matter :) Let's keep it one and add to directive/options token.

[bundlemon]

BundleMon

Files updated (1)

Status	Path	Size	Limits
✅	demo/browser/main.(hash).js	332.4KB (-37B -0.01%)	+10%

Unchanged files (4)

Status	Path	Size	Limits
✅	demo/browser/vendor.(hash).js	204.86KB	+10%
✅	demo/browser/runtime.(hash).js	34.83KB	+10%
✅	demo/browser/polyfills.(hash).js	19.92KB	+10%
✅	demo/browser/scripts.(hash).js	14.92KB	+10%

Total files change -36B -0.01%

Groups updated (1)

Status	Path	Size	Limits
✅	demo/browser/*..js	2.21MB (+773B +0.03%)	-

Final result: ✅

View report in BundleMon website ➡️

---

Current branch size history | Target branch size history

[codecov]

Codecov Report

Base: 61.46% // Head: 61.46% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (0458215) compared to base (86500d7).
Patch coverage: 42.85% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3141      +/-   ##
==========================================
- Coverage   61.46%   61.46%   -0.01%     
==========================================
  Files        1534     1534              
  Lines       18107    18107              
  Branches     2524     2524              
==========================================
- Hits        11130    11129       -1     
- Misses       6517     6518       +1     
  Partials      460      460              

Flag	Coverage Δ
addon-charts	72.46% <ø> (ø)
addon-doc	50.00% <ø> (ø)
addon-editor	51.23% <ø> (ø)
addon-mobile	62.31% <ø> (ø)
addon-table	60.70% <ø> (ø)
addon-tablebars	98.07% <ø> (ø)
cdk	39.90% <ø> (ø)
core	77.02% <42.85%> (-0.04%)	⬇️
kit	73.02% <ø> (ø)
summary	61.46% <42.85%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
.../directives/dropdown/dropdown-options.directive.ts	52.94% <0.00%> (-5.89%)	⬇️
...directives/dropdown/dropdown-position.directive.ts	36.36% <33.33%> (ø)
...cts/core/directives/dropdown/dropdown.component.ts	41.66% <50.00%> (ø)
...ives/dropdown/dropdown-position-sided.directive.ts	42.85% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[bondarvladislave]

@waterplea
I don't know how to add a translation in projects/demo/src/modules/components/abstract/dropdown-documentation/dropdown-documentation.template.html
phrase Dropdown vertical offset

[waterplea]

@waterplea
I don't know how to add a translation in projects/demo/src/modules/components/abstract/dropdown-documentation/dropdown-documentation.template.html
phrase Dropdown vertical offset

We only support English docs now, don't have resources for multiple languages, so don't worry about it.

[waterplea]

@bondarvladislave will you finish this PR? In discussion we figured out there's no need for 2 values because only one is used at any given time.

[bondarvladislave]

@waterplea

tuiDropdownSided uses two at once. And they are needed, I would like to avoid this vertical shift while increasing the horizontal one. That's why I need both.

It's just that in cases other than tuiDropdownSided it works in a very non-obvious way for the user, and therefore I would not want to add it to the Demo for everyone

[waterplea]

@bondarvladislave I'm not sure horizontal offset makes sense for traditional dropdown, if more complicated positioning is required, it's best to make your own PositionAccessor. As for vertical offset in case of sided dropdown, we need to think. This slight offset we currently have is meant to compensate for padding to align things like this:

Although currently it doesn't work that well, especially since that padding changes, depending on the size of DataList. Maybe we can have it a separate option for TuiDropdownPositionSidedDirective?

[bondarvladislave]

@waterplea Should I create a separate file for sided dropdown options or combine with all options this way?

Then [tuiDropdownOffset] will do nothing on sided because only sidedVerticalOffset and sidedHorizontalOffset will be used in the calculations.
And I don’t know if it is necessary and where exactly to describe it in the documentation and show it in the demo.

[waterplea]

How about we only make single offset in this PR, use it for vertical in regular and horizontal in sided, and we will address sided vertical position in another PR?

[bondarvladislave]

@waterplea
I hope I understood you correctly.
I removed the vertical calculations for the side opening.
What confuses me is that this will change this component for everyone who uses it at the moment, the side ones will be shifted 4px vertically.
And about another PR, will you do it yourself? Because I don't see a solution yet.

[waterplea]

@waterplea
I hope I understood you correctly.
I removed the vertical calculations for the side opening.
What confuses me is that this will change this component for everyone who uses it at the moment, the side ones will be shifted 4px vertically.
And about another PR, will you do it yourself? Because I don't see a solution yet.

Yes, I'll do the other one myself next week. We will release new version on Monday, then merge your PR and I'll cover the sided offset so there will not be a version where vertical offset is wrong.

[bondarvladislave]

@waterplea
Should I do something else?
I received your changes via rebase.

[well-done-bot]

[waterplea]

@waterplea Should I do something else? I received your changes via rebase.

No, I was just sick for a couple of days. Thank you for this PR.