-
Notifications
You must be signed in to change notification settings - Fork 464
PoC for allowing developers to control the defaults #2739
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
Conversation
|
this will apply to all popups and for entire lifecycle of the app? If so, I think this will not solve the problem totally, is normal to have popups with different configurations inside an application, that I said, I would say that forcing the same value for all popups aren't the best solution |
|
I agree it won't solve all use cases and that isn't my intention here. The intention is to make it possible for developers to override the default values. It also doesn't need to be restricted to just Popups. This could reduce the friction for some developers entirely. |
|
Let's first ensure the Defaults are documented and ensure developers how to modify the values and what the expected results should be. We are still in the transition period for Popup where Developers are reporting "Issues" that are really just them asking questions about how to migrate their Popup v1 to Popup v2. After developers have become accustomed to the new implementation of Popup v2, let's seek advice on ways we can improve it. For example, there are already ways for developers to create a base class that overrides our defaults: class PopupNoPadding : Popup
{
public PopupNoPadding()
{
Padding = 0;
}
} |
|
For padding yes this is possible but not for things like |
|
Fair point. Personally, I'd like to encourage developers to use a |
People might want to customize the popup for each individual instance, which could lead to multiple APIs for customization in different locations and more code to manage. One way to address this is to make the customization globally. I remember seeing something similar in the XF era, where styles were used to override values. We could modify the existing implementation to allow developers to customize the popup’s values using styles. Here’s a sample: take a look at the calendar. Pay attention to the The values are applied through this property. |
Although developers still need |
Not that I'm aware of. The return value is |
|
There's the scenario of having a Popup without a backing view model. Currently the Popup class has the ability to call close with a return value. |
|
Developers can create an instance with their default settings once and reuse it in their code. Setting some magic value through the whole app may lead to more issues rather then individual customization of each Popup. |
|
As a developer that uses popups, I would love to be able to set a few defaults at the application level. Our app is primarily used by people with motor control issues (tremor, jerkiness, etc), so I want to be able to just disable the "CanBeDismissedByTappingOutsideOfPopup" property application-wide, rather than needing to remember to either pass |
|
Closing in in favor of #2759 |
Description of Change
This is not the typical approach to proposing change but I find it helps me to follow through with seeing the change and impact of that change.
This is an idea that I have had for a while but the recent Popup bug reports around
CanBeDismissedByTappingOutsideOfPopuphas prompted an investigation into feasibility. The proposal is to make it possible for developers to modify our Defaults classes through the Options class when initialising the toolkit. Therefore it keeps it relatively safe knowing that the defaults cannot change at runtime.I have tried to group the defaults under feature specific classes to make it easier to navigate for a developer but I am keen to hear feedback on this.
I appreciate that it could be effort to add in all potential defaults but my thinking is to use this as an excuse to learn more about source generation and have that do the work that you see in the bulk of this PR. We could then place an attribute
[ConfigurableDefault("FeatureName")]and let the magic happen.It would then result in developers being able to call something like:
Thoughts?
Linked Issues
PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information