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

triggers to type + showTrigger #2

Closed
wants to merge 1 commit into from
Closed

triggers to type + showTrigger #2

wants to merge 1 commit into from

Conversation

Roman991
Copy link
Contributor

@Roman991 Roman991 commented Nov 18, 2023

This is just an untested proposal to review some design choices, feel free to close it or reply with your choice, so I can make a tested PR

  1. to add more consistency I've renamed trigger to showTrigger in module's config (as in template config).
  2. enums are inconvenient in template. For design you have to add NgxFloatUiTriggers do something like.
    import {  NgxFloatUiTriggers } from 'ngx-float-ui';
    triggerHover = NgxFloatUiTriggers.hover;

..................
<div
    [showTrigger]="triggerHover"
></div>

vs using types where the type check is the same

<div
    showTrigger="hover"
></div>

PS all README examples use [trigger] instead of [showTrigger]

@tonysamperi
Copy link
Owner

tonysamperi commented Nov 18, 2023

There is the floatUiLoose for lazy people who like hardcoding strings everywhere!

For the second part, that's a breaking change, I'm not inclined to add breaking changes just less than a week from publish. And it doesn't give any real improvement.

@Roman991
Copy link
Contributor Author

its ok, but examples need to be fixed, i'll update the readme

@tonysamperi
Copy link
Owner

Don't bother, I'll do it myself, so that I can mention the floatUiLoose in the readme!

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants