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

Make passing entry to DirectEntryScroll optional #702

Closed
wants to merge 2 commits into from

Conversation

@fireclawthefox
Copy link
Contributor

commented Aug 2, 2019

Made passing an exiting DirectEntry while initializing optional
Added function to set an Entry

Make passing entry optional
Made passing an exiting DirectEntry while initializing optional
Added function to set an Entry
@rdb

This comment has been minimized.

Copy link
Member

commented Aug 3, 2019

I'm just not sure that it should call destroy() on the old entry when assigning a new one, since this class isn't what created the entry to begin with. Thoughts?

@fireclawthefox

This comment has been minimized.

Copy link
Contributor Author

commented Aug 3, 2019

Hm, yeah, probably the function name doesn't really describes this part of the function well. Maybe check if we already have an entry and output an error or warning would be better. For another way, we'd probably need to enhance the class further to be able to hold more than one entry at the same time.

@rdb

This comment has been minimized.

Copy link
Member

commented Aug 4, 2019

I think it's fine, but I'm just suggesting that perhaps the .destroy() call shouldn't be there, but I'm happy to hear arguments on why it should.

@Moguri Moguri added the directgui label Aug 6, 2019

Fix set entry behavior
Removed destroy call and use notify.error call instead
Added clearEntry for convenience to remove a prevously set entry

@rdb rdb self-assigned this Aug 14, 2019

@rdb rdb added this to the 1.10.4 milestone Aug 14, 2019

@rdb rdb closed this in 24d48d8 Aug 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.