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: add a signal base hostBinding function #81

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

LcsGa
Copy link
Contributor

@LcsGa LcsGa commented Sep 26, 2023

As the title of the pull request suggests I added a hostBinding function that can be used to bind properties like @HostBinding would, but to signals (either writable or readonly).

As you would do to properties bound with the decorator, you are still able to set/update/mutate the signal if it is writable.

This is my very first OSS contribution, I hope I didn't mess up 😅

@wanoo21
Copy link
Contributor

wanoo21 commented Sep 27, 2023

It's so cool! I was just thinking about a function like this a few days ago. This is an awesome PR.

@LcsGa
Copy link
Contributor Author

LcsGa commented Sep 27, 2023

It's so cool! I was just thinking about a function like this a few days ago. This is an awesome PR.

Glad you like it 😄!

@tomalaforge
Copy link
Collaborator

nice one 👍
well documented and well tested.

@LcsGa
Copy link
Contributor Author

LcsGa commented Sep 28, 2023

nice one 👍
well documented and well tested.

Thanks! 😁

@nx-cloud
Copy link

nx-cloud bot commented Oct 2, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit cd953c4. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


🟥 Failed Commands
nx affected --target=build --parallel=3 --exclude=docs
✅ Successfully ran 3 targets

Sent with 💌 from NxCloud.

Copy link
Collaborator

@nartc nartc left a comment

Choose a reason for hiding this comment

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

LGTM. Can you rebase?

@nartc
Copy link
Collaborator

nartc commented Oct 5, 2023

@all-contributors please add @LcsGa for code

@allcontributors
Copy link
Contributor

@nartc

I've put up a pull request to add @LcsGa! 🎉

@LcsGa
Copy link
Contributor Author

LcsGa commented Oct 5, 2023

LGTM. Can you rebase?

Yes I can! But do you mean rebasing in order to have a single commit or to resolve conflicts or both (I use merge way more than rebase, except for when I want a clean branch, so I prefer asking 😅)?

@nartc
Copy link
Collaborator

nartc commented Oct 5, 2023

@LcsGa both

@LcsGa LcsGa force-pushed the feature/signal-host-binding branch from cbcce97 to cd953c4 Compare October 5, 2023 19:07
@LcsGa
Copy link
Contributor Author

LcsGa commented Oct 5, 2023

@nartc Done! I hope I didn't do it wrong 😅

@nartc nartc merged commit ee5f8aa into ngxtension:main Oct 5, 2023
4 of 5 checks passed
@nartc
Copy link
Collaborator

nartc commented Oct 5, 2023

@LcsGa Thanks for the contribution!

@LcsGa
Copy link
Contributor Author

LcsGa commented Oct 5, 2023

@LcsGa Thanks for the contribution!

With pleasure 😄

@LcsGa LcsGa deleted the feature/signal-host-binding branch October 5, 2023 20:59
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.

4 participants