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

Replace config(_:) to onActivate(_:) #273

Merged
merged 4 commits into from Mar 15, 2023
Merged

Replace config(_:) to onActivate(_:) #273

merged 4 commits into from Mar 15, 2023

Conversation

gmlwhdtjd
Copy link
Contributor

The current implementation of config(:) is very simple and powerful.
However, this implementation sometimes results in unintended execution of the config block passed to config(
:)
Also, because of the powerful implementation, it can sometimes be used in unintended ways.

So I created a new onActivate(_:) method, which solves the existing problem by storing the onActivate block in the view layout and deferring the execution of the block to the activation step.

@gmlwhdtjd gmlwhdtjd self-assigned this Feb 27, 2023
@gmlwhdtjd gmlwhdtjd added the enhancement New feature or request label Feb 27, 2023
Copy link
Collaborator

@oozoofrog oozoofrog left a comment

Choose a reason for hiding this comment

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

I cannot provide any justifiable reasons for this being disallowed.

@gmlwhdtjd
Copy link
Contributor Author

I cannot provide any justifiable reasons for this being disallowed.

@oozoofrog
Actually, I'm not sure where layoutWillActivate() should be executed.

The way it is now, it's hard to add a method like layoutDidActivate() in the future, so we need to think about how to solve this problem and see if there is a better way.

@oozoofrog
Copy link
Collaborator

i don't have any reasons for this not allowed.

I cannot provide any justifiable reasons for this being disallowed.

@oozoofrog Actually, I'm not sure where layoutWillActivate() should be executed.

The way it is now, it's hard to add a method like layoutDidActivate() in the future, so we need to think about how to solve this problem and see if there is a better way.

may be we need create one or more sandboxing area in Activation for some lazy logics. we'll find a way. as always.

@gmlwhdtjd gmlwhdtjd merged commit 9af45bf into develop Mar 15, 2023
@gmlwhdtjd gmlwhdtjd deleted the replace-config branch March 15, 2023 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants