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

Marking Observable properties from ReactiveObject and ReactiveRecord … #3695

Merged
merged 7 commits into from Dec 15, 2023
Merged

Marking Observable properties from ReactiveObject and ReactiveRecord … #3695

merged 7 commits into from Dec 15, 2023

Conversation

Micha-kun
Copy link
Contributor

…as no browsable, no displayable, no autogenerateable and no autofilterable for Winforms control databindings (and possibly for WPF controls too)

Fixes #3694

What kind of change does this PR introduce?
Assign new attributes on Observable properties for better hiding from Winforms/WPF

What is the current behavior?
Those properties are autogenerated as controls

What is the new behavior?
Those properties are ignored as controls

What might this PR break?

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:

Michael-Jorge Gómez Campos added 4 commits December 15, 2023 08:51
@Micha-kun
Copy link
Contributor Author

What's failing? I didn't touch any test :/

Thnxs

@ChrisPulman
Copy link
Member

What's failing? I didn't touch any test :/

Thnxs

You need to run the tests and update the API verified files, the following needs to be added, but you just need to copy the discovered to the verified to complete this task

[System.ComponentModel.Browsable(false)]
[System.ComponentModel.DataAnnotations.Display(AutoGenerateField=false, AutoGenerateFilter=false, Order=-1)]

and

namespace XamlGeneratedNamespace
{
    public sealed class GeneratedInternalTypeHelper : System.Windows.Markup.InternalTypeHelper
    {
        public GeneratedInternalTypeHelper() { }
        protected override void AddEventHandler(System.Reflection.EventInfo eventInfo, object target, System.Delegate handler) { }
        protected override System.Delegate CreateDelegate(System.Type delegateType, object target, string handler) { }
        protected override object CreateInstance(System.Type type, System.Globalization.CultureInfo culture) { }
        protected override object GetPropertyValue(System.Reflection.PropertyInfo propertyInfo, object target, System.Globalization.CultureInfo culture) { }
        protected override void SetPropertyValue(System.Reflection.PropertyInfo propertyInfo, object target, object value, System.Globalization.CultureInfo culture) { }
    }

@Micha-kun
Copy link
Contributor Author

What's failing? I didn't touch any test :/
Thnxs

You need to run the tests and update the API verified files, the following needs to be added, but you just need to copy the discovered to the verified to complete this task

[System.ComponentModel.Browsable(false)]
[System.ComponentModel.DataAnnotations.Display(AutoGenerateField=false, AutoGenerateFilter=false, Order=-1)]

and

namespace XamlGeneratedNamespace
{
    public sealed class GeneratedInternalTypeHelper : System.Windows.Markup.InternalTypeHelper
    {
        public GeneratedInternalTypeHelper() { }
        protected override void AddEventHandler(System.Reflection.EventInfo eventInfo, object target, System.Delegate handler) { }
        protected override System.Delegate CreateDelegate(System.Type delegateType, object target, string handler) { }
        protected override object CreateInstance(System.Type type, System.Globalization.CultureInfo culture) { }
        protected override object GetPropertyValue(System.Reflection.PropertyInfo propertyInfo, object target, System.Globalization.CultureInfo culture) { }
        protected override void SetPropertyValue(System.Reflection.PropertyInfo propertyInfo, object target, object value, System.Globalization.CultureInfo culture) { }
    }

Where are the API verified files to update? Sorry, it's my first time and don't know how to do this :)

image

This page says it should exist some file called APIApprovals.{TestName}.approved.txt as replaceable, where are they found? Where to put the replacement?

Thanks

@ChrisPulman
Copy link
Member

What's failing? I didn't touch any test :/
Thnxs

You need to run the tests and update the API verified files, the following needs to be added, but you just need to copy the discovered to the verified to complete this task

[System.ComponentModel.Browsable(false)]
[System.ComponentModel.DataAnnotations.Display(AutoGenerateField=false, AutoGenerateFilter=false, Order=-1)]

and

namespace XamlGeneratedNamespace
{
    public sealed class GeneratedInternalTypeHelper : System.Windows.Markup.InternalTypeHelper
    {
        public GeneratedInternalTypeHelper() { }
        protected override void AddEventHandler(System.Reflection.EventInfo eventInfo, object target, System.Delegate handler) { }
        protected override System.Delegate CreateDelegate(System.Type delegateType, object target, string handler) { }
        protected override object CreateInstance(System.Type type, System.Globalization.CultureInfo culture) { }
        protected override object GetPropertyValue(System.Reflection.PropertyInfo propertyInfo, object target, System.Globalization.CultureInfo culture) { }
        protected override void SetPropertyValue(System.Reflection.PropertyInfo propertyInfo, object target, object value, System.Globalization.CultureInfo culture) { }
    }

Where are the API verified files to update? Sorry, it's my first time and don't know how to do this :)

image

This page says it should exist some file called APIApprovals.{TestName}.approved.txt as replaceable, where are they found? Where to put the replacement?

Thanks

For me I use VS Code, when the test's run, VS Code launches with a Compare window, the left is discovered and the right verified, copy the contents of the discovered into the verified to make identical and save, run the tests again and all should pass

@ChrisPulman
Copy link
Member

If your still unable to get the test's to pass, I could create a branch for you to cherry pick from with the changes

@Micha-kun
Copy link
Contributor Author

Ok, updated verified files!

@Micha-kun
Copy link
Contributor Author

YYESS!

Copy link

codecov bot commented Dec 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (14821ae) 58.38% compared to head (3801d13) 58.65%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3695      +/-   ##
==========================================
+ Coverage   58.38%   58.65%   +0.27%     
==========================================
  Files         160      160              
  Lines        5774     5774              
  Branches     1028     1028              
==========================================
+ Hits         3371     3387      +16     
+ Misses       2015     2002      -13     
+ Partials      388      385       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ChrisPulman ChrisPulman merged commit 65377aa into reactiveui:main Dec 15, 2023
3 checks passed
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Mark ReactiveObject IObservable properties with DisplayAttribute for Winforms
3 participants