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

feat: switch to signal inputs #1835

Merged

Conversation

arturovt
Copy link
Contributor

This commit switches inputs to signal inputs to be compatible with zoneless change detection because everything should be a signal primitive. As such, its changes may be caught by the execution context. Every time a signal changes, it would schedule a change detection event, even in zoneless mode.

I didn't update @Output() to output because we still use .observed properties.

@arturovt arturovt marked this pull request as ready for review April 10, 2024 17:54

export const getFormat = (format?: QuillFormat, configFormat?: QuillFormat): QuillFormat => {
const passedFormat = format || configFormat
return passedFormat || 'html'
}

export function raf$() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice to use fat arrow functions

Copy link
Owner

@KillerCodeMonkey KillerCodeMonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And CI is failing coverage now.

maybe test your request animation helper.

or better the destroy callback

This commit switches inputs to signal inputs to be compatible with zoneless change detection
because everything should be a signal primitive. As such, its changes may be caught by the
execution context. Every time a signal changes, it would schedule a change detection event,
even in zoneless mode.

I didn't update `@Output()` to `output` because we still use `.observed` properties.
@arturovt
Copy link
Contributor Author

No matter what tests I add the coverage remains the same.

@KillerCodeMonkey
Copy link
Owner

i will check it thank you for the pr

@KillerCodeMonkey KillerCodeMonkey merged commit 3b12cdf into KillerCodeMonkey:master Apr 15, 2024
1 check failed
@arturovt arturovt deleted the feat/signal-inputs branch April 15, 2024 06:31
@KillerCodeMonkey
Copy link
Owner

FYI: it is the coverage for branches

if you run the test -> check the coverage folder there you have the text, json and html coverage reports

@andrei-varchanka
Copy link

now we get the error
image

@KillerCodeMonkey
Copy link
Owner

you need angular 17

my demo repo is up to date https://killercodemonkey.github.io/ngx-quill-example/
and it is working there.. like the ci is working in master

@arturovt
Copy link
Contributor Author

@andrei-varchanka, you don't need to upgrade to the latest version if you're staying with Angular versions lower than 17, as the latest changes are only compatible with ng17.

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.

3 participants