Update WebForms Control Tab to visualize DataBinding #716

Merged
merged 36 commits into from Feb 3, 2014

Conversation

Projects
None yet
3 participants
@grahammendick

DataBinding is the Web Forms equivalent to Model Binding in MVC. There are two different types of DataBinding in Web Forms for which Glimpse can meaningfully harvest data. The first is pre .NET 4.5 and uses DataSource Controls and the second is post .NET 4.5 and uses Model Binding. The aim of this PR is to capture the parameters used during DataBinding and for the data displayed to be consistent across both scenarios.

The initial proposal and subsequent discussion can be found under the Glimpse dev group post entitled 'Re: Glimpse Web Forms'. But for completeness I'll summarize the implementation.

A DataBoundControlAdapter is attached to all DataBoundControls. If pre .NET 4.5 DataBinding is used the parameters are extracted from the DataSource Control Parameters in the DataBinding listener attached in the Adapter. If post .NET 4.5 DataBinding is used the parameters are extracted from the ModelBindingContext in a ModelBinder Inspector. Once extracted, the parameters are stored in the HttpContext by the Adapter for subsequent retrieval by The ControlTree tab. PageLifeCycleMessages are used to display the parameters within the corresponding Page event.

A test page has been added to the WingTips sample, http://localhost:55555/DataBindingTests/ListView.aspx, that demonstrates a ListView DataBound using both types of DataBinding and involving a range of DataSource Controls, Parameters and ValueProviders.

grahammendick added some commits Jan 18, 2014

Adapter tweaks
Safer to add listener from generic instead of calling base.OnInit;
prevents base.OnInit being called second time from InnerAdapter
Qualified data bind params with event name
Used the Page Life Cycle messages to determine the event when the data
bind occurred
Reformatted mutliple databind within same event
Switch out list and replace with dictionary with int key if there are
multiple databindes for a single event. Makes the display nicer as per
glimpse dev group suggestion from anthony
Use start times only
Duration is internal to .NET and doesn't work for calculating times.
Start is DateTime.Now set by glimpse and works for boundary checking
Can't get event check to work
I'll ask anthony how he did it in the hud
Switch DateTime.Now to TimerStrategy
When running outside debug the DateTime.Now were all evaluated as equal
and so the matching of events failed.
Changed to use Gliimpse TimerStrategy as this is more accurate. Will
check with anthony how to avoid calling deprecated method
Changed to list for better display
As per Anthony's advice, switching from a dictionary to a list makes the
Key take up less room in the display and gives the parameter Values more
room
Aligned DataSource parms with model binding
Used reflection to get the DefaultProperty from the Parameter. This
matches with the ModelName for model binding
Aligned model binding and data source
Removed the 'Parameter' and 'ValueProvider' from type names and renamed
display column from Type to Source. This makes the data identical across
irrespective of whether model binding or data source used
Removed attribute
Base class Parameter has DefaultProperty attribute so no need for
Attribute test
@avanderhoorn

This comment has been minimized.

Show comment
Hide comment
@avanderhoorn

avanderhoorn Jan 28, 2014

Member

Great work to @grahammendick and @nycdotnet I'm going to try and get onto working through this PR this afternoon/tomorrow. Looks really good and represents a huge amount of work!!! This will see the WebForms package go to 1.1.

Also as food for thought, we need to think about how we can get more eyes on this webforms work. Its such awesome and more people who are doing webforms should know about.

Member

avanderhoorn commented Jan 28, 2014

Great work to @grahammendick and @nycdotnet I'm going to try and get onto working through this PR this afternoon/tomorrow. Looks really good and represents a huge amount of work!!! This will see the WebForms package go to 1.1.

Also as food for thought, we need to think about how we can get more eyes on this webforms work. Its such awesome and more people who are doing webforms should know about.

+ {
+#if NET45Plus
+ DataBoundControl.CallingDataMethods += DataBoundControl_CallingDataMethods;
+#endif

This comment has been minimized.

@grahammendick

grahammendick Jan 28, 2014

CallingDataMethods is for post 4.5 ModelBinding only

@grahammendick

grahammendick Jan 28, 2014

CallingDataMethods is for post 4.5 ModelBinding only

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Just to understand, both the CallingDataMethods and DataBinding are used in 4.5+? Just to catch me up whats the one line summary of what one does aspose to the other?

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Just to understand, both the CallingDataMethods and DataBinding are used in 4.5+? Just to catch me up whats the one line summary of what one does aspose to the other?

This comment has been minimized.

@nycdotnet

nycdotnet Jan 31, 2014

Contributor

CallingDataMethods is the event called by the "new" .NET 4.5 modelbinding stuff. DataBinding is the event called by pretty much every other form of data binding in WebForms.

@nycdotnet

nycdotnet Jan 31, 2014

Contributor

CallingDataMethods is the event called by the "new" .NET 4.5 modelbinding stuff. DataBinding is the event called by pretty much every other form of data binding in WebForms.

This comment has been minimized.

@grahammendick

grahammendick Jan 31, 2014

Yes, CallingDataMethods is the new event in 4.5 model binding - it allows you to provide a custom object on which your method will be called, allowing you to get code out of the code behind.
Yes, DataBinding is called by all forms of data binding, including model binding.

@grahammendick

grahammendick Jan 31, 2014

Yes, CallingDataMethods is the new event in 4.5 model binding - it allows you to provide a custom object on which your method will be called, allowing you to get code out of the code behind.
Yes, DataBinding is called by all forms of data binding, including model binding.

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Is there any UI difference that we show for DataBinging vs ModelBinding? Is so whats the page that I can access to test that?

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Is there any UI difference that we show for DataBinging vs ModelBinding? Is so whats the page that I can access to test that?

This comment has been minimized.

@grahammendick

grahammendick Jan 31, 2014

By design I kept the UI the same for both DataBinding and ModelBinding. However, with the code change we agreed, where we show the Type of parameter, e.g., Select, OrderBy, we're introducing a difference in the UI. Once I've put that code in you'll be able to see the difference on the ListView,aspx test page because there's both a Data and a Model Bound ListView there.

@grahammendick

grahammendick Jan 31, 2014

By design I kept the UI the same for both DataBinding and ModelBinding. However, with the code change we agreed, where we show the Type of parameter, e.g., Select, OrderBy, we're introducing a difference in the UI. Once I've put that code in you'll be able to see the difference on the ListView,aspx test page because there's both a Data and a Model Bound ListView there.

+#if NET45Plus
+ protected void DataBoundControl_CallingDataMethods(object sender, CallingDataMethodsEventArgs e)
+ {
+ HttpContext.Current.Items["_GlimpseWebFormModelBinding"] = new DataBindParameterModel(Offset);

This comment has been minimized.

@grahammendick

grahammendick Jan 28, 2014

Add a place holder to HttpContext, which will subsequently be populated by the ModelBinder Inspector

@grahammendick

grahammendick Jan 28, 2014

Add a place holder to HttpContext, which will subsequently be populated by the ModelBinder Inspector

+ {
+ var parameterModel = new DataBindParameterModel(Offset);
+ foreach (var parameters in GetParameters())
+ {

This comment has been minimized.

@grahammendick

grahammendick Jan 28, 2014

This part is for pre 4.5 DataBinding using DataSourceControls

@grahammendick

grahammendick Jan 28, 2014

This part is for pre 4.5 DataBinding using DataSourceControls

+ }
+#if NET45Plus
+ if (HttpContext.Current.Items.Contains("_GlimpseWebFormModelBinding"))
+ {

This comment has been minimized.

@grahammendick

grahammendick Jan 28, 2014

This part is for post 4.5 Model Binding

@grahammendick

grahammendick Jan 28, 2014

This part is for post 4.5 Model Binding

+ {
+ name = parameter.GetType().GetProperty(defaultPropertyAttribute.Name).GetValue(parameter, null) as string;
+ }
+ name = name ?? parameter.Name;

This comment has been minimized.

@grahammendick

grahammendick Jan 28, 2014

Extract the item source (e.g., ControlID, QueryStringField) by calling the default property of the Parameter, obtained from the DefaultPropertyAttribute that decorates native Parameters. Even custom Parameters are guaranteed to have a DefaultPropertyAttribute because the base Parameter class specifiies one, namely 'DefaultValue'. If this base one is found then it's ignored in favour of using the method parameter name instead.

@grahammendick

grahammendick Jan 28, 2014

Extract the item source (e.g., ControlID, QueryStringField) by calling the default property of the Parameter, obtained from the DefaultPropertyAttribute that decorates native Parameters. Even custom Parameters are guaranteed to have a DefaultPropertyAttribute because the base Parameter class specifiies one, namely 'DefaultValue'. If this base one is found then it's ignored in favour of using the method parameter name instead.

+ name = parameter.GetType().GetProperty(defaultPropertyAttribute.Name).GetValue(parameter, null) as string;
+ }
+ name = name ?? parameter.Name;
+ parameterModel.DataBindParameters.Add(new DataBindParameter(name, parameter.GetType().Name.Replace("Parameter", null), values[parameter.Name]));

This comment has been minimized.

@grahammendick

grahammendick Jan 28, 2014

Remove the string 'Parameter' from the type name so that the Source display column is the same for both pre and post 4.5 data binding, e.g., show Control rather than ControlParameter

@grahammendick

grahammendick Jan 28, 2014

Remove the string 'Parameter' from the type name so that the Source display column is the same for both pre and post 4.5 data binding, e.g., show Control rather than ControlParameter

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Are we adding this so we can access the data later, are there any side effects from adding to the parameter list?

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Are we adding this so we can access the data later, are there any side effects from adding to the parameter list?

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Scratch that.... just noticed its our model we are adding to... Don't mind me.

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Scratch that.... just noticed its our model we are adding to... Don't mind me.

+ HttpContext.Current.Items.Remove("_GlimpseWebFormModelBinding");
+ }
+#endif
+ if (parameterModel.DataBindParameters.Count > 0)

This comment has been minimized.

@grahammendick

grahammendick Jan 28, 2014

Parameters can be empty (but not null), e.g., DataBoundControls that use neither DataSourceControls nor Model Binding

@grahammendick

grahammendick Jan 28, 2014

Parameters can be empty (but not null), e.g., DataBoundControls that use neither DataSourceControls nor Model Binding

+ }
+ }
+
+ public class DataBoundControlAdapter<TControlAdapter> : DataBoundControlAdapter

This comment has been minimized.

@grahammendick

grahammendick Jan 28, 2014

Inherits from non-generic version of DataBoundControlAdapter so it can share the implementation details

@grahammendick

grahammendick Jan 28, 2014

Inherits from non-generic version of DataBoundControlAdapter so it can share the implementation details

+ DataBoundControl.CallingDataMethods += DataBoundControl_CallingDataMethods;
+#endif
+ DataBoundControl.DataBinding += DataBoundControl_DataBinding;
+ OnInitInfo.Invoke(InnerAdapter, new object[] { e });

This comment has been minimized.

@grahammendick

grahammendick Jan 28, 2014

Safer to manually attach listeners than call base.OnInit because there's already one call to base.Onit in the reflection call and so need to be careful it's not called twice

@grahammendick

grahammendick Jan 28, 2014

Safer to manually attach listeners than call base.OnInit because there's already one call to base.Onit in the reflection call and so need to be careful it's not called twice

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Again thinking about memory, are we unhooking the listeners if we need to? You know more about the context than me so we might be good here.

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Again thinking about memory, are we unhooking the listeners if we need to? You know more about the context than me so we might be good here.

This comment has been minimized.

@grahammendick

grahammendick Jan 31, 2014

There's no need to unhook listeners, .NET sorts this out.

@grahammendick

grahammendick Jan 31, 2014

There's no need to unhook listeners, .NET sorts this out.

@avanderhoorn

This comment has been minimized.

Show comment
Hide comment
@avanderhoorn

avanderhoorn Jan 28, 2014

Thanks for this!

Thanks for this!

{
- trace.IsEnabled = true;
+ HttpContext.Current.Items["_GlimpseWebFormDataBinding"] = new Dictionary<string, List<DataBindParameterModel>>();

This comment has been minimized.

@grahammendick

grahammendick Jan 28, 2014

Creating placeholder here saves cluttering up the DataBoundControlAdapter

@grahammendick

grahammendick Jan 28, 2014

Creating placeholder here saves cluttering up the DataBoundControlAdapter

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Just wondering why we need to set the at this point? By the time GetData isn't everything over by then?

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Just wondering why we need to set the at this point? By the time GetData isn't everything over by then?

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Don't worry, just remembered that we are getting called twice.

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Don't worry, just remembered that we are getting called twice.

}
- };

This comment has been minimized.

@grahammendick

grahammendick Jan 28, 2014

Look for any application-specific Adapters for Controls inheriting from DataBoundControl. For each found, replace with generic-wrapped version. Without this step then our DataBoundControlAdapter would be skipped if there was a more specific application-specific one, e.g., a ListViewAdapter.

@grahammendick

grahammendick Jan 28, 2014

Look for any application-specific Adapters for Controls inheriting from DataBoundControl. For each found, replace with generic-wrapped version. Without this step then our DataBoundControlAdapter would be skipped if there was a more specific application-specific one, e.g., a ListViewAdapter.

+ if (!adapters.Contains(dataBoundControlType.AssemblyQualifiedName))
+ {
+ adapters.Add(dataBoundControlType.AssemblyQualifiedName, dataBoundControlAdapterType.AssemblyQualifiedName);
+ }

This comment has been minimized.

@grahammendick

grahammendick Jan 28, 2014

Add a catch-all non generic-wrapper Adapter for all DataBoundControls if we've not already registered a generic version in the above loop.

@grahammendick

grahammendick Jan 28, 2014

Add a catch-all non generic-wrapper Adapter for all DataBoundControls if we've not already registered a generic version in the above loop.

+ <OrderGroupsByParameters>
+ <asp:FormParameter Name="OrderGroups" FormField="SomeFormKey"/>
+ </OrderGroupsByParameters>
+ </asp:LinqDataSource>

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 30, 2014

Member

Is there any way to capture as part of the output the parameter type? In the above case it would be Order, Select, etc.

@avanderhoorn

avanderhoorn Jan 30, 2014

Member

Is there any way to capture as part of the output the parameter type? In the above case it would be Order, Select, etc.

This comment has been minimized.

@grahammendick

grahammendick Jan 30, 2014

That's definitely doable but it would mean that pre 4.5 and post 4.5 data binding display would diverge.

@grahammendick

grahammendick Jan 30, 2014

That's definitely doable but it would mean that pre 4.5 and post 4.5 data binding display would diverge.

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 30, 2014

Member

I think thats ok, because in this case its just an extra column. I think it will add a lot of value.

@avanderhoorn

avanderhoorn Jan 30, 2014

Member

I think thats ok, because in this case its just an extra column. I think it will add a lot of value.

This comment has been minimized.

@grahammendick

grahammendick Jan 30, 2014

Ok, what's the best way to handle this so the column doesn't appear for the model binding parameters? Would you use 2 different models or is there a better way?

@grahammendick

grahammendick Jan 30, 2014

Ok, what's the best way to handle this so the column doesn't appear for the model binding parameters? Would you use 2 different models or is there a better way?

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 30, 2014

Member

Probably best to have 2 models, or one that inherits from the other. Glimpse does also support an array of arrays data structure, which will layout just like an array of objects. In this case, position 0 in the root array holds the heads you want and the rest hold the values.

@avanderhoorn

avanderhoorn Jan 30, 2014

Member

Probably best to have 2 models, or one that inherits from the other. Glimpse does also support an array of arrays data structure, which will layout just like an array of objects. In this case, position 0 in the root array holds the heads you want and the rest hold the values.

+ <FilterParameters>
+ <asp:QueryStringParameter Name="Filter" QueryStringField="Test"/>
+ </FilterParameters>
+ </asp:SqlDataSource>

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 30, 2014

Member

Looking at this and how we are mapping the attributes here into the UI. Do you think it would be better to rename the Name column to Field? Or does that not work in other cases.
sqldatasource

@avanderhoorn

avanderhoorn Jan 30, 2014

Member

Looking at this and how we are mapping the attributes here into the UI. Do you think it would be better to rename the Name column to Field? Or does that not work in other cases.
sqldatasource

This comment has been minimized.

@grahammendick

grahammendick Jan 30, 2014

Field or Key would be better. Which do you prefer?

@grahammendick

grahammendick Jan 30, 2014

Field or Key would be better. Which do you prefer?

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 30, 2014

Member

Field matches what the attributes are. If we go with Field does this work elsewhere?

@avanderhoorn

avanderhoorn Jan 30, 2014

Member

Field matches what the attributes are. If we go with Field does this work elsewhere?

This comment has been minimized.

@grahammendick

grahammendick Jan 30, 2014

Not completely. The column refers to how to retrieve the data from the source, e.g., for Control it would be the Control ID; for Session it would be the Sesson key; for QueryString it would be the query string key; for Cookie it would be the Cookie Name

@grahammendick

grahammendick Jan 30, 2014

Not completely. The column refers to how to retrieve the data from the source, e.g., for Control it would be the Control ID; for Session it would be the Sesson key; for QueryString it would be the query string key; for Cookie it would be the Cookie Name

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 30, 2014

Member

This is though since ID and Name have other connotations and meanings. Field does seem to be the thing that is most common across even thoughs - Like in the sample its QueryStringField for QueryString and SessionField for Session.

@avanderhoorn

avanderhoorn Jan 30, 2014

Member

This is though since ID and Name have other connotations and meanings. Field does seem to be the thing that is most common across even thoughs - Like in the sample its QueryStringField for QueryString and SessionField for Session.

+<browsers>
+ <browser refID="Default">
+ <controlAdapters>
+ <adapter controlType="System.Web.UI.WebControls.ListView" adapterType="WingtipToys.DataBindingTests.ListViewAdapter, WingtipToys" />

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 30, 2014

Member

I'm assuming we have added this for test purposes? That being the case, I think its a good addition.

@avanderhoorn

avanderhoorn Jan 30, 2014

Member

I'm assuming we have added this for test purposes? That being the case, I think its a good addition.

This comment has been minimized.

@grahammendick

grahammendick Jan 30, 2014

Yes, just for testing purposes

@grahammendick

grahammendick Jan 30, 2014

Yes, just for testing purposes

+
+ public string DataBindParametersTitle
+ {
+ get { return DataBindParameters != null ? "DataBind Parameters" : null; }

This comment has been minimized.

@nycdotnet

nycdotnet Jan 31, 2014

Contributor

This is really nitpicky, but maybe just "Databinding" would be better here. It's about the same length as "Viewstate" and still gets the idea across. What do you think, Graham?

@nycdotnet

nycdotnet Jan 31, 2014

Contributor

This is really nitpicky, but maybe just "Databinding" would be better here. It's about the same length as "Viewstate" and still gets the idea across. What do you think, Graham?

This comment has been minimized.

@grahammendick

grahammendick Jan 31, 2014

Nitpicky is good! I like the change

@grahammendick

grahammendick Jan 31, 2014

Nitpicky is good! I like the change

This comment has been minimized.

+ yield return linqDataSource.GroupByParameters;
+ yield return linqDataSource.OrderGroupsByParameters;
+ }
+ }

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

This is a smallish thing, but it could be worth switching over from yield and to a list that is populated and returned. I know the syntax might be a little more verbose, but we have had several memory leaks introduced over time and they all had to do with yields. Is that ok?

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

This is a smallish thing, but it could be worth switching over from yield and to a list that is populated and returned. I know the syntax might be a little more verbose, but we have had several memory leaks introduced over time and they all had to do with yields. Is that ok?

This comment has been minimized.

@grahammendick

grahammendick Jan 31, 2014

No problem, I need to change this method anyway to return the Type of the Parameter as agreed earlier

@grahammendick

grahammendick Jan 31, 2014

No problem, I need to change this method anyway to return the Type of the Parameter as agreed earlier

+
+namespace Glimpse.WebForms.Inspector
+{
+ public class DataBoundControlAdapter : ControlAdapter

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

So here we are generally using the same pattern we came up with for ViewStatePageAdapter, which means that we wrap other implementations, etc?

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

So here we are generally using the same pattern we came up with for ViewStatePageAdapter, which means that we wrap other implementations, etc?

This comment has been minimized.

@grahammendick

grahammendick Jan 31, 2014

Yes, the same pattern

@grahammendick

grahammendick Jan 31, 2014

Yes, the same pattern

+ }
+ };
+
+ traceContextVerifyStartMethod.Invoke(trace, null);

This comment has been minimized.

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

I'm assuming that you have kept the logic the same and just refactored this up to here to make it a little cleaner? Or was it just the differ.

@avanderhoorn

avanderhoorn Jan 31, 2014

Member

I'm assuming that you have kept the logic the same and just refactored this up to here to make it a little cleaner? Or was it just the differ.

This comment has been minimized.

@grahammendick

grahammendick Jan 31, 2014

The logic's the same but I had to move it because of a weird .NET bug. Once I added the new Adapters then the tab stopped working - can't remember what exactly the symptoms were - but switching the tracing and the Adapters around fixed it (you can see for yourself if you switch them back)

@grahammendick

grahammendick Jan 31, 2014

The logic's the same but I had to move it because of a weird .NET bug. Once I added the new Adapters then the tab stopped working - can't remember what exactly the symptoms were - but switching the tracing and the Adapters around fixed it (you can see for yourself if you switch them back)

return null;
}
- context.Logger.Debug("ControlTree Tab Finial Run - {0}", HttpContext.Current.Request.RawUrl);
+ context.Logger.Debug("ControlTree Tab Final Run - {0}", HttpContext.Current.Request.RawUrl);

This comment has been minimized.

@avanderhoorn

This comment has been minimized.

Show comment
Hide comment
@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Ok got through it all and it looks great! Just one thing I've been wondering, I'm thinking that GetData on ControlTree is starting to get a bit heavy... do you think its worth refactoring it a bit to try and neaten it up now we know that we are up and running?

Member

avanderhoorn commented Jan 31, 2014

Ok got through it all and it looks great! Just one thing I've been wondering, I'm thinking that GetData on ControlTree is starting to get a bit heavy... do you think its worth refactoring it a bit to try and neaten it up now we know that we are up and running?

@grahammendick

This comment has been minimized.

Show comment
Hide comment
@grahammendick

grahammendick Jan 31, 2014

Sure, how about I move the Adapter registration into a separate method?

Sure, how about I move the Adapter registration into a separate method?

@grahammendick

This comment has been minimized.

Show comment
Hide comment
@grahammendick

grahammendick Jan 31, 2014

I've made the code amendments as per the review comments.
When I added in the Select, OrderBy to qualify the Data Bind Parameters it made the display a bit wide and can cause a horizontal scrollbar. Is this ok?

I've made the code amendments as per the review comments.
When I added in the Select, OrderBy to qualify the Data Bind Parameters it made the display a bit wide and can cause a horizontal scrollbar. Is this ok?

@avanderhoorn

This comment has been minimized.

Show comment
Hide comment
@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Looking fantastic. I think having the Type columns adds a lot of addition use.

I know this is a really minor thing, but I'm thinking that the order of the parameters probably makes more sense being Type, Source, Field & Value or Field, Value, Type & Source. What do you think?

I think with that last change we should be good to bring it in.

Member

avanderhoorn commented Jan 31, 2014

Looking fantastic. I think having the Type columns adds a lot of addition use.

I know this is a really minor thing, but I'm thinking that the order of the parameters probably makes more sense being Type, Source, Field & Value or Field, Value, Type & Source. What do you think?

I think with that last change we should be good to bring it in.

@avanderhoorn

This comment has been minimized.

Show comment
Hide comment
@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Just had one last thought, do we think it could be worth identifying if the default value has been used and what that is? We do a similar thing in the Routes tab, although I don't like how we show it very much.
routes

Member

avanderhoorn commented Jan 31, 2014

Just had one last thought, do we think it could be worth identifying if the default value has been used and what that is? We do a similar thing in the Routes tab, although I don't like how we show it very much.
routes

@grahammendick

This comment has been minimized.

Show comment
Hide comment
@grahammendick

grahammendick Jan 31, 2014

Is there any easy way to change the order of the columns around? Remember, I'm using a base Parameter class with the extra column in the subclass as we discussed
I can do Type, Source, Field & Value I guess, but not the other with the current subclass approach?
I guess I could use a Converter to do the switch?

Is there any easy way to change the order of the columns around? Remember, I'm using a base Parameter class with the extra column in the subclass as we discussed
I can do Type, Source, Field & Value I guess, but not the other with the current subclass approach?
I guess I could use a Converter to do the switch?

@avanderhoorn

This comment has been minimized.

Show comment
Hide comment
@avanderhoorn

avanderhoorn Jan 31, 2014

Member

Passively the order of the Properties or dictionary items is the main
default factor.

On 31 January 2014 17:26, grahammendick notifications@github.com wrote:

Is there any easy way to change the order of the columns around? Remember,
I'm using a base Parameter class with the extra column in the subclass as
we discussed

Reply to this email directly or view it on GitHubhttps://github.com/Glimpse/Glimpse/pull/716#issuecomment-33848881
.

Member

avanderhoorn commented Jan 31, 2014

Passively the order of the Properties or dictionary items is the main
default factor.

On 31 January 2014 17:26, grahammendick notifications@github.com wrote:

Is there any easy way to change the order of the columns around? Remember,
I'm using a base Parameter class with the extra column in the subclass as
we discussed

Reply to this email directly or view it on GitHubhttps://github.com/Glimpse/Glimpse/pull/716#issuecomment-33848881
.

@grahammendick

This comment has been minimized.

Show comment
Hide comment
@grahammendick

grahammendick Jan 31, 2014

Showing the DefaultValue isn't straightforward. It's stored as a string but the actual value is an object. For example, if it's a DateTime then the DefaultValue would be in the format yyyy/mm/dd, e.g., 2007/12/24 (so its locale independent) but the actual value would be a DateTime. So, if the string wasn't converted into the correct type, then it wouldn't match up in the UI. It is possible to convert it because the Type is also a property of the Parameter, but it's not trivial.
Also, how would this be shown in the UI. I guess we'd have to go with a Converter and use TabSections like your Route Converter, otherwise how else can you get a space in the column title?

Showing the DefaultValue isn't straightforward. It's stored as a string but the actual value is an object. For example, if it's a DateTime then the DefaultValue would be in the format yyyy/mm/dd, e.g., 2007/12/24 (so its locale independent) but the actual value would be a DateTime. So, if the string wasn't converted into the correct type, then it wouldn't match up in the UI. It is possible to convert it because the Type is also a property of the Parameter, but it's not trivial.
Also, how would this be shown in the UI. I guess we'd have to go with a Converter and use TabSections like your Route Converter, otherwise how else can you get a space in the column title?

@avanderhoorn

This comment has been minimized.

Show comment
Hide comment
@avanderhoorn

avanderhoorn Feb 1, 2014

Member

Humm let's leave it out for the time being then.

On Friday, January 31, 2014, grahammendick notifications@github.com wrote:

Showing the DefaultValue isn't straightforward. It's stored as a string
but the actual value is an object. For example, if it's a DateTime then the
DefaultValue would be in the format yyyy/mm/dd, e.g., 2007/12/24 (so its
locale independent) but the actual value would be a DateTime. So, if the
string wasn't converted into the correct type, then it wouldn't match up in
the UI. It is possible to convert it because the Type is also a property of
the Parameter, but it's not trivial.
Also, how would this be shown in the UI. I guess we'd have to go with a
Converter and use TabSections like your Route Converter, otherwise how else
can you get a space in the column title?

Reply to this email directly or view it on GitHubhttps://github.com/Glimpse/Glimpse/pull/716#issuecomment-33851615
.

Member

avanderhoorn commented Feb 1, 2014

Humm let's leave it out for the time being then.

On Friday, January 31, 2014, grahammendick notifications@github.com wrote:

Showing the DefaultValue isn't straightforward. It's stored as a string
but the actual value is an object. For example, if it's a DateTime then the
DefaultValue would be in the format yyyy/mm/dd, e.g., 2007/12/24 (so its
locale independent) but the actual value would be a DateTime. So, if the
string wasn't converted into the correct type, then it wouldn't match up in
the UI. It is possible to convert it because the Type is also a property of
the Parameter, but it's not trivial.
Also, how would this be shown in the UI. I guess we'd have to go with a
Converter and use TabSections like your Route Converter, otherwise how else
can you get a space in the column title?

Reply to this email directly or view it on GitHubhttps://github.com/Glimpse/Glimpse/pull/716#issuecomment-33851615
.

@avanderhoorn

This comment has been minimized.

Show comment
Hide comment
@avanderhoorn

avanderhoorn Feb 1, 2014

Member

So am I good to go on pulling this in?

On Friday, January 31, 2014, grahammendick notifications@github.com wrote:

Showing the DefaultValue isn't straightforward. It's stored as a string
but the actual value is an object. For example, if it's a DateTime then the
DefaultValue would be in the format yyyy/mm/dd, e.g., 2007/12/24 (so its
locale independent) but the actual value would be a DateTime. So, if the
string wasn't converted into the correct type, then it wouldn't match up in
the UI. It is possible to convert it because the Type is also a property of
the Parameter, but it's not trivial.
Also, how would this be shown in the UI. I guess we'd have to go with a
Converter and use TabSections like your Route Converter, otherwise how else
can you get a space in the column title?

Reply to this email directly or view it on GitHubhttps://github.com/Glimpse/Glimpse/pull/716#issuecomment-33851615
.

Member

avanderhoorn commented Feb 1, 2014

So am I good to go on pulling this in?

On Friday, January 31, 2014, grahammendick notifications@github.com wrote:

Showing the DefaultValue isn't straightforward. It's stored as a string
but the actual value is an object. For example, if it's a DateTime then the
DefaultValue would be in the format yyyy/mm/dd, e.g., 2007/12/24 (so its
locale independent) but the actual value would be a DateTime. So, if the
string wasn't converted into the correct type, then it wouldn't match up in
the UI. It is possible to convert it because the Type is also a property of
the Parameter, but it's not trivial.
Also, how would this be shown in the UI. I guess we'd have to go with a
Converter and use TabSections like your Route Converter, otherwise how else
can you get a space in the column title?

Reply to this email directly or view it on GitHubhttps://github.com/Glimpse/Glimpse/pull/716#issuecomment-33851615
.

grahammendick added some commits Feb 1, 2014

Improved column names in display
Replace dictionary with List so that columns have specific names, Event
and Parameters instead of Key and Value.
Also renamed sub columns to Index and Values instead of Key and Value.
Also, added row to UI if data bound without parameters so the event when
binding occurred is displayed.
Display DefaultValue for DataSource data binding
Convert the DefaultValue to an object so it matches the format of the
actual value ,e.g., DateTime defaults will be entered in format
yyyy/mm/dd (so it works across different locales).
Also added a DataBindParameter converter to reorder columns in display
@grahammendick

This comment has been minimized.

Show comment
Hide comment
@grahammendick

grahammendick Feb 1, 2014

I've made a few improvements:

  1. Added DefaultValue to the UI for pre 4.5 Parameters
  2. Switched out Dictionary to improve the naming of columns in the UI
  3. Display DataBind Page Event even if there aren't any parameters. Steve should like this because it gives information even when manually DataBinding. You can see an example of this for the ManualDataBindListView in the ListView.aspx

Let me know what you think.

I've made a few improvements:

  1. Added DefaultValue to the UI for pre 4.5 Parameters
  2. Switched out Dictionary to improve the naming of columns in the UI
  3. Display DataBind Page Event even if there aren't any parameters. Steve should like this because it gives information even when manually DataBinding. You can see an example of this for the ManualDataBindListView in the ListView.aspx

Let me know what you think.

@avanderhoorn avanderhoorn self-assigned this Feb 3, 2014

@avanderhoorn avanderhoorn added this to the vNext milestone Feb 3, 2014

@avanderhoorn

This comment has been minimized.

Show comment
Hide comment
@avanderhoorn

avanderhoorn Feb 3, 2014

Member

Great work mate!!! Just checking out the changes now and will pull it in :D

Member

avanderhoorn commented Feb 3, 2014

Great work mate!!! Just checking out the changes now and will pull it in :D

avanderhoorn added a commit that referenced this pull request Feb 3, 2014

@avanderhoorn avanderhoorn merged commit d2b806b into Glimpse:master Feb 3, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment