RadMenu Client-Side Item Add vs NavigationScripts.js (*kablooey*)

[PureCode]

Hi,

I am adding a crapload of items to a RadMenu from javascript.

A few issues show up, and kill my efforts.

Consider the following piece of javascript, from your NavigationScripts.js:

	_createChildListElement: function()
	{
	throw Error.notImplemeneted();
	},
	_createDomElement: function()
	{
	throw Error.notImplemented();
	},

Issue 1
Why are both of these functions not implemented? (Mind you, they have not been implemented, as far as i can go back, since the Q1 2008 release.)

Issue 2
_createChildListElement, unimplemented as it is, has a typo in it, throwing a nice 'Element does not support...' error message instead of the intended not implemented message. Slight detail being that in the very next function, it is not misspelled. This too has been the case since at least Q1 2008.

That about wraps up the issues within the Telerik code.

My problem is as follows, I am adding a load of menu items, from client-side (and only client-side), using a for-next loop which retrieves its information from an already defined menu structure in a RadTreeView.

	function MenuEditorUpdatePreviewMenu()
	{
	// Get the preview menu.
	var editorTestMenu = $find($telerik.$("[ID$='MenuEditorTestRadMenu']")[0].id);
	// Get the treeview
	var editorTreeView = $find($telerik.$("[ID$='MenuEditorRadTreeView']")[0].id);
	// Get all nodes from the tree.
	var nodes = editorTreeView.get_allNodes();
	// Start tracking changes to the menu.
	editorTestMenu.trackChanges();
	// Iterate through all the nodes.
	for (var index = 0; index < nodes.length; index++)
	{
	// Determine if it is a root node.
bo	if (nodes[index].get_level() == 0)
mb	{
bo	menuItem = new Telerik.Web.UI.RadMenuItem();
mb
bo	menuItem.set_text(nodes[index].get_text());
mb	menuItem.set_value(nodes[index].get_value());
bo	menuItem.set_navigateUrl(nodes[index].get_navigateUrl());
mb
bo	editorTestMenu.get_items().add(menuItem);
mb	}
	}
	// Commit the changes to the menu.
	editorTestMenu.commitChanges();
	}

The above code bombs out with that 'not implemented' for the _createChildListElement function in NavigateScripts.js (well, it shows the 'Element does not support...' error message due to the typo, which confused me for about three seconds or so).

I know it is not the for-next or the if-then statement, even if i simply try to insert a single item (eg, without the for and if code around it), it bombs out. I went as far as removing everything except getting the menu control, trackchanges, adding a single RadMenuItem with a text of "TEST" and, commitchanges, even that bombs out with the same error. I even directly copied code from your demos (where it works fine) to do this adding of items, with, you guessed it, the same error.

Both the RadMenu and the RadTreeView controls are found correctly as well. The get_allNodes() does indeed return all the nodes in the RadTreeView.

The issue originates from the following Telerik javascript (NavigationScripts.js):

	_notifyPropertyChanged: function(_72, _73)	What's with the empty function?
	{
	},
	_childInserting: function(_74, _75, _76)	Ditto.
	{
	},
	_childInserted: function(_77, _78, _79)
	{
	if(!_79._childControlsCreated)
	{
	return;
	}
	if(!_79.get_element())
	{
	return;
	}
	var _7a = _78._createDomElement();
	var _7b = _79.get_childListElement();	This always returns 'null'.
	if(!_7b)	So, this evaluates to 'false' per default.
	{
	_7b = _79._createChildListElement();	Causing it to call the function that is not implemented.
	}
	var _7c = _78.get_nextSibling();
	var _7d = _7c ? _7c.get_element() : null;
	_79.get_childListElement().insertBefore(_7a, _7d);
	if(!_78.get_element())
	{
	_78.set_element(_7a);
	_78._initializeRenderedItem();
	}
	else
	{
	_78.set_element(_7a);
	}
	},

I should note, ALL of the other code works 100%, just adding information from tree nodes (text, value, url) to a new RadMenuItem and adding the item to a RadMenu bombs out in this manner (heck, everything i try to add to that RadMenu bombs out in this manner).

I'd love to know why, since at least Q1 2008, this _clientCreateListElement (and _createDomElement right below it, for that matter) has not been implemented, and, way more importantly, why is my code blowing up on that fact, while your demos WORK? (And i am keeping in mind that i have noticed in the past that some of your demo stuff (webmail demo anyone?) use MODIFIED Telerik assemblies (in the case of the webmail demo, you commented out the background color for RadPanes in the Hay skin, and i can see why, you owe me new eyes now, while it is not commented out in ANY of the release assemblies.)

Regards,

Mike

[PureCode]

Hi,

Little update regarding the above.

After some experimenting (read: trial & error 'just do some **** and see what happens'), i discovered that if i add a 'dummy' menu item to the RadMenu (which starts off devoid of items, since it is populated entirely from the client-side), everything works as it should.

Conclusion, the error in the previous post exists because the RadMenuItemCollection (or whatever it is called), is simply not there.

In my logic, if i create a RadMenu and decide to leave it empty for some reason or another, this collection really SHOULD exist. It seems that it doesn't by default and this caused me to waste at least an hour or two on trying to figure out what the hell was going on (in retrospect, i should've thought of this the moment it happened, but, as stated, in my logic, that collection HAS to be there by default).

As thus, i consider this a bug within the Telerik assembly. So, hereby, one bug report.

Regards,

Mike

[T. Tsonev]

Hi Mike,

Please, accept my apologies for wasting your time with such annoying bug. I agree that an empty menu should be fully functional regardless if it has items or not. We'll take care of this issue, but for the moment the dummy item looks like the only workaround. We'll try to fix it in time for the upcoming service pack that is due next week.

The _createChildListElement and the similar are the equivalent of abstract methods in JavaScript. Not all controls override them, as some of them rely on elements rendered from the server. This is actually the cause of the bug in question.

The empty methods are needed when we want to skip the actions performed by the base class. Same as overriding a virtual method and leaving it empty.

As a token of our gratitude for your involvement, your Telerik points have been updated.

Best wishes,
Tsvetomir Tsonev
the Telerik team

Instantly find answers to your questions on the new Telerik Support Portal.
Check out the tips for optimizing your support resource searches.

[PureCode]

Hi Tsvetomir,

Many thanks for the swift reply. I ended up using the 'dummy' item to accomplish fixes for a few 'issues' with the RadMenu (these are client-side, javascript 'issues').

1. Without items, the menu doesn't render at all.
2. After adding an item and removing it, the menu is visible but only 16px high, instead of the 26px (including border) it is when an item is present.
3. And, of course, to 'initialize' the item collection.

I think that even if no menu items are present, the menu should render to the width and height specified for it. The height property is ignored entirely (hell, even setting the height to 24px by setting height using your javascript code for this, as well as directly on the element, doesn't do a thing) as it is. Obviously, for the sake of 'looks', i expect an empty RadMenu to render, at the correct width and height. The dummy item fixes all of this, so at least there is an easy work-around.

In the same manner, i use a dummy item in the RadTreeView that displays the hierarchy of the menu (this particular bit of code (99% client-side) is a menu editor) just to get it to display the first dotted line at the top (which, too, i think should be there by default if there are no nodes in it).

All this 'dummy' code is bloating my javascript pretty hard since i need to remove it and re-add it based upon the adding and removing of nodes/items for the menu and treeview. I can live with all of that for this particular demonstration application, but in a production environment it'd be a pain (and certainly not pass code-review based upon our standards and even more, our clients standards). So, a fix for the RadMenu (the one that needed the most 'extra code' for the 'dummy item') is very welcome and appreciated.

Please allow me to use this opportunity to more or less ask a question and show you something i found quite amusing and disturbing at the same time.

Consider the following snippet of javascript code from your RadDock control:

	dockingZoneHitTest: function(e)
	{
	var _53;
	var _54 = null;
	var _55 = this.get_dockZones();
	for (var i = 0; i < _55.length; i++)
	{
	_53 = _55[i];
	if (_53.hitTest(this, e))
	{
	_54 = _53;
	}
	}
	return _54;
	}

Two vars are created, one of which explicitly as 'null' (_54), then another one that is filled with the dock zones. At this point, you guys go into a for-next loop where an iteration is done on the var holding the dock zones. Each one is subsequently tested for a 'hit'. The before mentioned var (_54) which was explicitly set to null is set to the other var when a 'hit' is detected. So, it now knows what zone was hit, yet the for-next continues until all zones are hit tested. After the for-next, the var _54 is returned, either with a dock zone in it, or as it was declared, null.

The problem (apart from some SERIOUSLY crappy code!) is that if _54 remains null (no zones were hit), the null returned doesn't 'register', in fact, it simply bombs out within the calling function. I do not understand why it does this (tested only under IE7 btw), but this is what we saw happening, and is how we discovered the above function.

So, the developer who was dealing with docks at that point in time (a little while back) wanted me to ask you what your developer was smoking when he/she coded the above function, and whether or not he can have some.

Joking aside, we, of course, had to fix this 'bug' (or whatever it is that is happening with that null bombing out), so here is the function my developer whipped up in a minute or two back then that fixes the odd 'bug' as well as forms somewhat eh.. 'nicer' code:

	dockingZoneHitTest: function(e)
	{
	for (var index in this.get_dockZones())
	{
	var zone = this.get_dockZones()[index];
	if (zone.hitTest(this, e))
	{
	return zone;
	}
	}
	return null;
	}

Note the difference, he does not declare any variables apart from one single variable that holds the current dock zone within the for-next (more like a for-each though), once it finds a hit on a zone, it immediately returns the zone. If nothing was hit, it explicitly returns null. Code-wise, simplicity itself and fixed that strange 'bug' as well as fixed that incredibly dodgy bit of code you guys have in there (RadDock.js).

So, that's just an FYI on that strange little 'bug' with that null var, as well as pointing out that the Telerik javascript is incredibly below par code-wise for this particular function (in our opinion, of course). Perhaps the replacement my developer quickly whipped up back then might be a good replacement.

Are there a lot more of these rather 'what the ..?' kind of functions in your javascript? Efficient code is very important, and the code used in this particular function is rather, well, 'amateurish' (although my developer had some other 'names' for it) and while not of high impact (how many dock zones is one going to have, right), there is a lot of stuff in your product that would REALLY impact efficiency if coded in a similar manner.

Don't regard this as an 'attack' on Telerik, since this is, so far, the only REALLY crap code we have run into within your javascript, albeit that some of the rest isn't all that fantastic either (this one is just plain ridiculous tho).

Thanks once more!

Regards,

Mike

[T. Tsonev]

Hi,

I fully sympathize with you, jumping through hoops shouldn't be necessary to achieve such trivial functionality. We'll do our best to resolve this issue in RadMenu and RadTreeView as soon us possible.

You really have a point about the dockingZoneHitTest function. I agree that not stopping the loop can be a major performance trap.  Your optimized version deals with this issue, but there are some potential issues with it.

First, using for(var x in y) is not recommended for arrays. Extending the Array prototype is common practice among not-so-unobtrusive JavaScript libraries (you know who you are) and you can get some very interesting results while iterating this way. Here is an example with a popular library taken directly from the Firebug console:

>>> a = [1, 2]
[1, 2]
>>> for (var i in a) { console.log(i); }
0
1
each
eachSlice
all
any
...

Also, caching local variables is very important performance-wise. More for some browsers, less for others. With this considerations the function can look like this:

dockingZoneHitTest: function(e)
{
    var dockZones = this.get_dockZones();
    for (var i = 0, length = dockZones.length; i < length; i++)
    {
        var zone = dockZones[i];
 
        if (zone.hitTest(this, e))
        {
            return zone;
        }
    }
 
    return null;
}

There is the slight detail that the optimized version will return the first match, while the original version will return the last matching zone. I'll talk with my colleagues about the intended behavior of the function, but my guess is that only one zone will match the particular coordinates.

We'll investigate the problem that occurs when this function returns null and will eventually add some form of error handling.

As for the overall code quality I can only say that we constantly refactor it and apply newly learned lessons. Otherwise we simply won't be able to develop and support our products properly. This is a constant effort with focus shifting from one control to another, so naturally some code is newer than other.

We value your feedback and don't treat it as attack. Constructive criticism is always welcome.

All the best,
Tsvetomir Tsonev
the Telerik team

Instantly find answers to your questions on the new Telerik Support Portal.
Check out the tips for optimizing your support resource searches.

[PureCode]

Hi Tsvetomir,

Once, again, thanks for the reply.

I agree that the 'code-style' use by my developer to replace your javascript doesn't entirely conform to our own internal standards. However, in the last years we have discovered that the use of so many 'var' keywords in large amounts of javascript significantly increases the filesizes, so it ends up in a choice between one 'optimization' versus another. I can't really say that i have seen a lot of issues with the usage of a 'var' to hold an array or directly using the array within, say, a for-next. The 'for-each' style is something that we use all over the place (and we have many tens of thousands of lines of javascript in our framework) in our scripts and, as long as it is properly tested, hasn't posed any problems, as far as i can see in our 'development-diaries', albeit that you are correct that sometimes the results are somewhat unpredictable.

All web developers wish that everything worked as it should in all browsers, and the incompatibilities between each browser is probably the biggest pain in the royal rear end we all deal with. Sadly, this isn't going to change, ever.

Working in the industry we cater to forces us to follow (often ridiculously) strict regulations, one of which is that we are forced to develop under Internet Explorer (at least we get to choose whether we use version 7 or 8) and THEN 'port' to other browsers for compatibility. My guess is that they simply want 'standard' development under a 'standard' browser, before the code/script gets mangled to be made compatible (eg, we always have to maintain two versions, one that is IE only, one that is cross-browser), just in case something blows up and they need a version that simply works out of the box in the market-leading browser.

We develop our framework using three product-suites, mainly to see which of these three 'market leading suites' is best suited for our purposes (which are scary, to say the least, thanks to said regulations as well as the needs our clients have). Each suite (in Teleriks case, it was still called 'prometheus', so, more or less a beta of the current suite) took anywhere between 4 and 8 months to get 'cleared' for use. Any time we implement a beta version of our framework at a client, a similar 'certification' is required, although that process is much faster since we, as a business, are certified to develop in this industry, although the time required depends on the specific implementation and how 'classified' the (clients) project is. In the past we have developed software for client projects that took more than a year to get 'cleared' for use, meaning that by the time the software got implemented at the client, it was already more than a year old (excluding development time). Thankfully this is not (usually) a problem since the average project of our clients can be anywhere from 5 to 12 years in length. We continue to develop (usually upgrade, sometimes, restart from scratch) the software for that entire period of time as well.

As for the docking zone hit test, i can't see any situation where dock zones would overlap. If my memory serves me correctly, your client-side code bases the hit detection on the location of the mouse pointer, not on the actual 'area' that a dragged dock covers (in which case one could have more than one hit, but then the routine would have to return an array of the hit dock zones, not the first or last).

I have to admit that we don't use your client-side code for docks, we have completely replaced it due to large amounts of problems surfacing when using your client-side code. Some of them, off the top of my head:

• The entire browser client area scrolling if the mouse hits the right or bottom of the client area
• An, in our opinion, badly designed way of doing the placeholders.
• The lack of a 'dock manager' control.
• The dock layout control being, more or less, totally useless, as it does practically nothing 'layout' related (and doesn't even show up in the actual webpage source at runtime, but that i could be totally wrong there).
• The mouse pointer 'loosing' the dragged dock (usually after the afore mentioned 'scrolling' issue, but also when moving the mouse rapidly).
• And probably a few somewhat more trivial things that i can't remember (it's been a while).

In the end, we decided that, in order to get rid of major issues like the scrolling, we would have to replace the entire client-side code for RadDock (we didn't want to replace the entire control set, so we do use the server-side code, albeit that it is heavily modified in the derived versions we use). For us it, thankfully, wasn't that hard to do and we even added a number of extra functionality. As a side note, our resulting javascript is slightly smaller than yours (a few hunderd lines or so), for the most part we do still use some of the functions you have in your 'core.js'.

Pretty much the same thing with your splitter controls, although the problem there was the sheer destruction of the page-layout since your splitters are anything but WYSIWYG. We actually integrated our custom dock and custom splitter into a single 'control set', allowing us to mix and match them where needed (while retaining the page-layout in the designer).

Earlier this year we finally got jQuery 'cleared' for use (yes, even if your 'cleared' suite already uses it, we still had to submit it and wait for it to clear.. as i said, often ridiculous regulations), and our replacement client-side code for RadDock is on the list to be 'jQueried' (albeit it not as a 'plugin' to jQuery, THAT did NOT 'clear', for whatever reason). Actually, i think at least of one of my developers is already working on that. RadGrid is way more important to us, so that one was the first and also sports entirely replaced (and jQuery based) client-side code now (with the functionality we really wanted, automatic column sizing (mostly) done the way the Microsoft Winforms grid does it), retaining all of your functionality and a bunch of new stuff added.

Out of all three suites, most of us like yours the best due to the fact that we CAN easily 'replace' stuff where needed without running into major issues elsewhere (a testament to the robustness of your server-side code, your competitors can learn a thing or two there). None of the three suites do exactly what we need (as is always the case), so we have to be able to easily add/remove/replace script and code where needed to make controls do what we need them to do, and Telerik definitely shines in that department (albeit that too much of your assembly is 'internal', but that is what reflection is for).

In the end, now that we are finally seeing the end of the initial development cycle for the framework approaching, it is pretty much clear that Telerik will be our choice to use within the framework. While it has been the most time-consuming to get to work correctly (for our purposes), it has also been the easiest to work with. The results of replacing client-side code, for example, are predictable and don't interfere with RAD controls that have little to no customization applied. Not so much with your competitors suites, nightmares, many of them.

The only 'issue' i have remaining on the list of pros/cons for Telerik is the (still) not-so-great skinning, as that particular side of your suite is HIGHLY unpredictable, even with our custom skin creation tool. However, i prefer somewhat 'lesser' skinning over 'much lesser' customizing abilities.

Anyways, to end this already way too long of a post, Telerik support has never failed us, and that is commendable since i tend to be pretty vocal when i run into issues that start costing too much time (money..). Being a pain in the rear and still getting the same support as anyone else, is quite rare (especially since the issues i tend to be vocal about are generally somewhat 'stranger' than the usual issues i see floating around these forums).

While i have 'issues' with some of your products (OpenAccess is (in our opinion) rather, eh.. crap, with a capital C, and your reporting doesn't support web-based report design (MAJOR issue, and a definite deal killer with my clients, we developed our own in the end), to name the two main ones), Telerik as a business understands that the community should 'drive' at least part of the development process, provides support in pretty much the same 'mindset' as we do, and the suite we are developing with (ASP.NET AJAX of course) is, in general, of high quality without a massive assembly (unlike some of your competitors, 16+ assemblies is just ridiculous). Sure, it is buggy in places, but those are easily fixed due to the ease with which we can modify almost anything within your assembly (the drop down calendar being a prime example, when we noticed it consistently falling outside of the client window in places, we applied a simple fix and the problem was gone).

Keep it up! If our framework becomes what it is intended to become, you'll be gaining a lot of rather large customers, and well deserved as well.

Regards,

Mike

[T. Tsonev]

Hi,

Verbosity is really something to be kept in check with JavaScript, as it affects performance. Of course correctness shouldn't suffer. Thus we tend to rely on tools like compressors to take care of it while we focus on the task at hand. By the way, jQuery offers very convenient functions like each that are very succinct.

All in all libraries like jQuery, dojo and others go a long way in making the web development more pleasant experience, but I can't deny that despite the improvements we still have a lot of code that targets specific browsers.

It sounds like you've gone through a lot of effort to adapt our controls for your project. It clearly shows that we have a lot to improve, but the fact that you've managed to do it is a great compliment to us.

I'll forward your feedback regarding the RadDock, RadSplitter, RadGrid and Reporting to my coallegues. Thank you for pointing out these issues.

We have some things going on the subject of skinning. First, there is the Visual Style Builder which aims to deliver easy skin customization. Read morea about it here:
http://blogs.telerik.com/blogs/09-05-19/meet_the_visual_style_builder_ctp.aspx

We also have a "simple" skin in the works that will be able to serve as a foundation for custom skins.

Thank you very much for taking the time to share your thoughts with us. This kind of deep and insightful feedback is rare and we value it.

Kind regards,
Tsvetomir Tsonev
the Telerik team

Instantly find answers to your questions on the new Telerik Support Portal.
Check out the tips for optimizing your support resource searches.

[PureCode]

Hi once more Tsvetomir,

Verbosity should indeed be kept in check within javascript (same goes for my replies and posts on this forum), however, readability is paramount. One needs to be able to assign a 'generic developer' to do maintenance or recode bits and pieces on any code/script without needing this person to spend their time figuring out what on earth the code is doing.

A good example is the script code you deliver with your trial downloads (and i assume, your non-source code included subscription downloads), while readable, it is a mystery (most of the time) what the variables are due to the slight bit of obfuscation you guys have applied. Not a big deal in general, but does require us to insert 'debugger;' statements into your javascript a lot to see what is going exactly. Variable names like _54, _1A, etc aren't exactly easy to deal with after all.

And yes, tools to minify javascript and such are great for 'compressing' the production versions of said javascript files. Even for that we wrote our own tools (most tools out there can't be 'cleared' for use, considering the amount of changes they make to the source files). We get the jQuery 'development' javascript slightly smaller than their 'production' javascript actually (but not by much of course).

I think our greatest asset as a business is the flexibility we can offer and apply in regards to using 'existing code', most of my developers are experts at decyphering legacy code, whereas i myself am more specialized at 'customizing' and 'optimizing' code. Being an engineer/business-man (as opposed to being a straight developer) i am, thankfully, able to deal with both sides of the coin.

We are discovering the power of jQuery rapidly. We never really went in 'deep and hard' with it because it simply wasn't cleared for use yet. Slowly we're re-implementing most of our custom javascript to use jQuery. One major example we just finished up is a complete replacement for dragging and dropping, including fully overriding your implementation.

It seems to me that if you guys would 'adapt' (read: re-code) your javascript for ASP.NET AJAX, your assembly would shrink considerably in size, since jQuery is capable of doing so much of the functionality you have inside your javascript in a much more straight-forward (and compatible) way. Especially your cross-browser javascript leaves much to be desired (IE vs FireFox is and remains a major pain in the rear with your javascript, but that is, in the end, not your fault). Your javascript also seems to be verbose in regards to the sheer amount you have of it, many functions doing the same thing as other functions, but with a slight difference in there somewhere. Of course, javascript is and remains a procedural scripting language, no matter what anyone claims (it's not OO, it never will be OO, and it cannot be OO due to the way it is designed and implemented, it sure is good at pretending to be OO though), which means we're always limited in some way or another. The transition from coding server-side (C#) to client-side coding (javascript) is a bit of a shell-shock to most of my developers and results often in time wasted because they try to do things the C# way and javascript becomes highly unpredictable in its results because of it.

Anyways, considering our expertise in optimizing your client-side code, i am more than happy to share our replacement script where i am able to (some of it is classified due to the functionality it adds to some of your controls, functionality that is industry-specific, but removal of such script/code is not an issue, it won't break anything).

If we can save you guys some time and effort translating your proprietary script to jQuery based equilavants, then that is an advantage to my business, Telerik and the users. Our jQuery based drag & drop script, for example, is WAY more 'efficient' than yours, and fixes most of the issues i mentioned in my previous post, and can be used anywhere within the Telerik namespace to replace your implementation with a few lines of script (and, in our case, overriding some of your functions, of course).

I think i could provide you with our entire RadDock replacement actually, it contains no industry-specific code as far as i am aware. Although some work would be required on our side to make it somewhat more 'Telerik' compatible in both code-style as well as implementation (since it also does the same work for the other two suites as well as our own libraries), but that should not be an issue.

I don't know how Telerik feels about such an offer, but i figured i'd throw it out there in case you would be interested (even if to just study what we did and how we did it). It would certainly be quicker than listing all the issues we found with your product suite (i have been compiling a rather massive 'book' on all of them for you, but going through all the development diaries, my own notes, and even the commenting inside our code/script is a VERY time consuming task and my time is limited of course, i do have a business to run).

Said 'issue list' is giant, and inevitably contains issues you have already fixed, i cannot check that of course. We have worked with your entire suite and know it inside out by now (at least, in combined knowledge of my people). We understand the limitations, we know where the somewhat more 'buggy' code is and we know, for the most part, how to fix these issues (some of the fixes, especially early ones, are rather hackish). This is the result of relentless experimenting, i force my developers and designers to experiment. Just try to DO things with it, anything, see what happens, document it. Apart from learning rapidly what one is working with, it is also an amazing QA process since they start finding the strangest bugs, and at the same time the limitations are exposed quickly as well, allowing for experimentation to remove those limitations, where needed.

So, there you have a bit more insight on how we work as a business, and how we ended up doing things with your product that probably few others have done in the past. In the end, it is still your product, with our own 'Telerik.Web.UI.Extended' library attached to it (making changes directly in a third party assembly is just not done in my opinion, and probably against your EULA as well, but i know for a fact that doesn't stop a lot of people from doing so anyway, in any case upgrading becomes a nightmare).

A few things that would be a definite help to us (and probably many others too):

• A version of your assembly without the embedded skins and scripts.

• More base controls (next to RadControl and RadWebControl).
  • RadScriptControl.
    
    • Full scripting support.
    • No skinning support
    • One with client-state support, one without.
    • One with postback support, one without.
  • RadSkinnedControl
    • Skinning support.
    • No scripting support.
    • One with client-state support, one without.
    • One with postback support, one without.

• Less 'internal' classes.
  • For example, expose the EmbeddedSkinAttribute, which would massively ease the use of (reflected) SkinRegistrar methods, since we had to jump through hoops-on-fire to get the above suggested base controls coded and working inside our extention library, requiring a LOT more custom code than really should have been needed.
  • Another example would be the ScriptRegistrar, which was a pain in the rear to reflect properly (and, more or less, required us to 'duplicate' code you already have in your assembly, but could not properly be reflected upon, goes for the SkinRegistrar as well).

• Extend some of the classes.
  • For example, the EmbeddedSkinAttribute (again), after exposing it, allow it to find skins embedded in separate assemblies, allowing us to bundle our custom skins into an assembly and rid our already way too large project solutions of dozens of skin files.

Apologies for inserting custom HTML into your reply box, but i found no way to do the multi-indentation other than creating it in Visual Studio and copy & paste it (although i experimented with this in an earlier post above with your code box).

For a little more insight, a small bit of code from our 'RadHeaderPane' control (derived from RadPane, but allows a header to be shown, as well as a (scrollable, or not) 'inner content panel' with fitting border, in any combination of the two, it also fixes the issue with RadPane being too wide in some situations (a single pixel drops out of the client window if one is on the right of the screen, for example, as visible in your WebMail demo)).

From the server-side rendering code:

	if (this.ShowHeader || (this.ShowHeader && this.UseContentPanel))
	{
	writer.Write(string.Format("<div id=\"{0}\" {1} style=\"{2}{3}{4}{5} overflow-y: hidden !important;\">",
	new object[]
	{
	string.Format("RAD_SPLITTER_PANE_CONTENT_{0}", this.ClientID),
	str,
	Helpers.InvokeBaseMethod<string>(this, base.GetType(), "GetWidthAttribute", null),
	Helpers.InvokeBaseMethod<string>(this, base.GetType(), "GetHeightAttribute", null),
	Helpers.InvokeBaseMethod<string>(this, base.GetType(), "GetBackColorStyle", null),
	Helpers.InvokeBaseMethod<string>(this, base.GetType(), "GetForeColorStyle", null)
	}));
	}

This is very similar to your own server-side code (note the missing 'GetScrollOverflowStyle' as compared to yours). However, we are forced to use the 'Helpers.InvokeBaseMethod' method because the 'GetWidth..', 'GetHeight..', etc methods inside your assembly (for RadPane) are declared as 'private'. While no big issue, it adds a lot of overhead since reflection has to be used to get the values we need.

For completeness, the actual 'InvokeBaseMethod':

	/// <summary>
	/// Invokes methods within the base class of an inherited class.
	/// </summary>
	/// <typeparam name="T">Specifies the type that is returned.</typeparam>
	/// <param name="object">The object on which to invoke the method.</param>
	/// <param name="base">The base type of the object.</param>
	/// <param name="method">The name of the method to invoke.</param>
	/// <param name="parameters">Any parameters that need to be passed to the invoked method.</param>
	/// <returns>The result of the method invocation, as the type specified.</returns>
	public static T InvokeBaseMethod<T>(object @object, Type @base, string method, params object[] parameters)
	{
	T result = default(T);
	BindingFlags bindingFlags = BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Public;
	Type baseType = @base.BaseType;
	MethodInfo methodInfo = baseType.GetMethod(method, bindingFlags);
	if (methodInfo == null)
	{
	throw new MissingMethodException(string.Format("Could not find method {0} in {1}.", method, @base.BaseType));
	}
	result = (T)methodInfo.Invoke(@object, parameters);
	return result;
	}

That particular method doesn't do anything special obviously, but it does show the overhead that is added to a derived control when one needs to use 'private' or 'internal' methods of the base control.

I am not even mentioning the fact that the header we added to the pane is based on the RadDock skin, which is not 'accessible' when using a RadPane (unless RadDock is used on the page the RadHeaderPane extention control is on). For this we developed a bit of code that reads the specified CSS from your assembly, dissects it, and fetches the bits we need to draw said header. Works very well, but again adds a lot of overhead and extra code.

Well, another big reply, apologies for taking up so much of your time. I appreciate the time you take to read and reply to my lengthy posts.

Regards,

Mike

[PureCode]

For those interested, the entire (rather crappy) set of code needed to extract CSS directly from the Telerik assembly as well as parse it into bite-sized bits:

	using System.Collections.Generic;
	using System.IO;
	using System.Reflection;
	using System.Text.RegularExpressions;
	using System.Web.UI;
	namespace Telerik.Web.UI.Extended
	{
	public class CssFiles
	{
	public static CssFile ExtractEmbeddedCss(string skin, string controlName)
	{
	return ExtractEmbeddedCss(skin, controlName, false);
	}
	public static CssFile ExtractEmbeddedCss(string skin, string controlName, bool baseCss)
	{
	string input = string.Empty;
	Assembly assembly = Assembly.GetAssembly(typeof(Telerik.Web.UI.RadWebControl));
	string CssResourceName;
	if (baseCss)
	{
	CssResourceName = string.Format("Telerik.Web.UI.Skins.{0}.css", controlName);
	}
	else
	{
	CssResourceName = string.Format("Telerik.Web.UI.Skins.{0}.{1}.{0}.css", skin, controlName);
	}
	using (StreamReader reader = new StreamReader(assembly.GetManifestResourceStream(CssResourceName)))
	{
	input = reader.ReadToEnd();
	}
	Regex ExtractCSS = new Regex(
	@"(?im-nsx)" +
	@"(?<class>^\.[0-9a-zA-Z_\,\.\:\s]+)\s*\r\n" +
	@"(?<open>\{)\s*\r\n" +
	@"(?:\s+(?<key>[a-zA-Z\-]+)\:\s*" +
	@"(?<value>.*)\;\s*\r\n)*" +
	@"(?<close>\})");
	MatchCollection CssClasses = ExtractCSS.Matches(input);
	CssFile LoadedCss = new CssFile(string.Format("{1}.{0}.css", skin, controlName));
	foreach (Match cssClass in CssClasses)
	{
	CssClass CssClass = new CssClass();
	CssClass.ClassName = cssClass.Groups["class"].ToString();
	for (int index = 0; index < cssClass.Groups["key"].Captures.Count; index++)
	{
	string cssValue = cssClass.Groups["value"].Captures[index].ToString().Replace("\"", "'");
	CssProperty CssCommand = new CssProperty(
	cssClass.Groups["key"].Captures[index].ToString(),
	cssValue);
	CssClass.CssCommands.Add(CssCommand);
	}
	LoadedCss.CssClasses.Add(CssClass);
	}
	return LoadedCss;
	}
	public class CssProperty
	{
	public CssProperty(string key, string value)
	{
	this.Key = key;
	this.Value = value;
	}
	public string Key { get; set; }
	public string Value { get; set; }
	}
	public class CssClass
	{
	public List<CssProperty> CssCommands;
	public CssClass()
	{
	CssCommands = new List<CssProperty>();
	}
	public string ClassName { get; set; }
	}
	public class CssFile
	{
	public List<CssClass> CssClasses;
	public CssFile()
	{
	CssClasses = new List<CssClass>();
	}
	public CssFile(string filename)
	{
	CssClasses = new List<CssClass>();
	this.Filename = filename;
	}
	public string Filename { get; set; }
	public string GetCssClassPropertyValue(string className, string property)
	{
	return GetCssClassPropertyValue(className, property, null);
	}
	public string GetCssClassPropertyValue(string className, string property, string ignore)
	{
	foreach (CssClass cssClass in CssClasses)
	{
	if (cssClass.ClassName.ToLowerInvariant().IndexOf(className.ToLowerInvariant()) != -1 &&
	(ignore.IsEmptyOrNull() || cssClass.ClassName.ToLowerInvariant().IndexOf(ignore.ToLowerInvariant()) == -1))
	{
	foreach(CssProperty cssCommand in cssClass.CssCommands)
	{
	if (cssCommand.Key.ToLowerInvariant() == property.ToLowerInvariant())
	{
	return cssCommand.Value;
	}
	}
	}
	}
	// Nothing found.
	return string.Empty;
	}
	public string GetWebResourceString(Control control, string skin, string fullCssName)
	{
	string WebResourceString = string.Empty;
	if (control.Page != null)
	{
	WebResourceString = Regex.Replace(
	fullCssName,
	"<%\\s*=\\s*WebResource\\([\"|\'](?<resourceName>[^\"]*)[\"|\']\\)\\s*%>",
	delegate(Match match) { return WebResourceEvaluator(match, skin, control); });
	}
	return WebResourceString;
	}
	private string WebResourceEvaluator(Match match, string skin, Control control)
	{
	// Limited to RadDock! Easy enough to specify which particular
	// RAD control one wants with some modification of this method.
	RadDock TempRadDock = new RadDock();
	TempRadDock.Skin = skin;
	string WebResourceString = ((Control)control).Page.ClientScript.GetWebResourceUrl(
	TempRadDock.GetType(), match.Groups["resourceName"].Value);
	return WebResourceString;
	}
	}
	}
	}

Also, it needs the following extension method (although one can easily use the standard MS provided one, albeit clunky):

	/// <summary>
	/// Provides an easier way of determining whether or not a
	/// string is null or empty.
	/// </summary>
	/// <returns>True if the string is null or empty, false if not.</returns>
	public static bool IsEmptyOrNull(this string @string)
	{
	return string.IsNullOrEmpty(@string);
	}

Note the rather 'heavy' (yet, pretty advanced, as far as i could tell from a Google search and finding a definite lack of working CSS parsing regular expressions, and even this one can be optimized to be better quite easily) regular expression used to parse the actual CSS classes, that's pretty much the heart of this code. Also note that it checks for both a double quote (") as well as a single quote ('), Telerik is anything but consistent in their use of quotes through-out the skins.

Another note would be that i had to pretty much limit the WebResourceEvaluator method to do RadDock only as the original actually contained some industry-specific code, for whatever reason (which i DO tend to find out from the developer who implemented this code into our libary, as it goes against our policies to do so in generic code like this). For the rest it is pretty much my code, mixed with some Telerik inspired code. It's pretty crappy and there is miles of space for optimizing, but as it is code that is rarely used, we never bothered. It works (as far as i am aware) perfectly on the Telerik skins though, any other CSS, especially the regular expression, it might work, it might not work, never tested on anything but Telerik skins.

This regular expression will not work on those CSS classes that stick the { symbol AFTER the class name definition, considering the idiocy of doing that in the first place (it is a REALLY bad coding-style that wouldn't pass any decent code-review, yet millions of noobs do it, and i am not going to help them make it even more common to mangle code in this manner).

Regards,

Mike

PS: Telerik, it seems that your code block doesn't like the double-quote, single quote, double-quote ("'") very much, a large part of the code block is seen as a string after it.

[T. Tsonev]

Hi Mike,

The original JavaScript code, along with the other source, comes with the subscription license. The compressed version is the only one we allow to be served to the actual users, as the compression also doubles as 'mild' obfuscation. Having the full source code is of great help when doing any customizations and that's one of the reasons we include it.

We bundle a tool for JavaScript compression along with the source code too. We didn't author our own, but your efforts in that area are worth our respect if you've managed to beat YUI compressor (I think that's what they use to compress jQuery).

We realize the potential of jQuery to improve both the speed and the size of our code. We try to make most of it and all new code that we author uses it in one way or another. It also has a big influence on the way we write code and I think it's very positive. JavaScript is indeed very different from C# and it takes time learn how to be effective. Getting used to prototypes, closures and functional programming can be very liberating, but the process is not easy for the average C# developer. Unfortunately, jQuery isn't a panacea and some of the most tricky parts of our code are here to say. One of the things it doesn't cover is quirks mode. It's not officially supported by jQuery and we handle most of the, ahem, quirks in our code.

I'll forward your suggestion about sharing your RadDock implementation to my manager. I guess that there are some legal implications here and I'm not really sure what to say besides thank you.

Providing an assembly without skins and scripts would be of limited use to most of our customers. For such cases we would recommend building the controls from source.

The base classes in the assembly are mostly there because we need to inherit from different framework classes - Control, WebControl, DataBoundControl, etc. I'm not sure that adding yet another layer of inheritance would be beneficial to anyone. Overriding EnableEmbeddedSkins and EnableEmbeddedScripts is a simpler solution that will give mostly the same result.

In this line of thoughts, I agree that we should expose more of the internal classes to inheritors. We preferred to have those internal, so we would not hurt users that rely on them if we decide to change them. As the library matures such changes become less likely and we can review this decision and expose most of the skin and script registration mechanism.

Point taken about the internal/private methods on the controls. Anything that can be of use to inheritors should be protected, not private.

Nice work with the CSS extraction tool. It's also possible to manually combine skins in external files, but having a tool is more flexible. Getting the tool done is also great exercise, especially when some regex magic is required.

Thank you again for sharing your thoughts with us.

Best wishes,
Tsvetomir Tsonev
the Telerik team

Instantly find answers to your questions on the new Telerik Support Portal.
Check out the tips for optimizing your support resource searches.

[PureCode]

Tsvetomir,

Thanks for the reply.

We potentially have a lot of code to share, considering that our framework replaces so much of your javascript (mainly for non-bug related issues, but more to add functionality and such as well as 'lighten the load' a little in places as your javascript is a little verbose in places). I am not a lawyer, so i cannot comment on any legal issues with such sharing, and the main reason i tend to discuss these things with Telerik first.

The CSS extraction code was an interesting bit of code to create. We needed CSS from one control to add to another control in order to form (kind of) a hybrid control. Our RadHeaderPane control pretty much combines your basic RadPane with a RadDock (albeit no RadDock code is used) in order to produce a Pane that has an internal header (which can have an internal content area). In fact, it was one of the first controls we made that doesn't override large portions of your javascript yet extends the functionality of the original RadPane by a fair amount. The total (uncompressed, commented and white-spaced) lines of javascript used for our RadHeaderPane control is 220 lines, and most of that is getters/setters. Server-Side code-wise it weighs in at 441 lines of code (again, with heavy commenting and white-spacing) and derives from RadPane, 99% of this is properties and rendering, the rest is the CSS extraction (using the helper code i posted above) and skinning stuff (in order to get the correct RadDock skin for the header portion). I believe it even supports your RadSkinManager. At the same time it gave us the added benefit of fixing that little issue with a 1px bit of RadPane being drawn outside of the screen. It's a good example on what's possible with your product suite without heavily overriding existing javascript and still create a majorly customized control without breaking the functionality Telerik provides, in relatively little time (about 4.5 hours total according to the logs, excluding the CSS extraction code (the RegEx was a ***** to create).

Do you think the CSS extraction code i posted above (with a fixed GetWebResource method) would be beneficial to the community if i posted it in the code library?

I was thinking the same for the RadHeaderPane control we created but it does override two Telerik javascript methods (_setSize and _splitterLoadedHandler), and on the server-side it does the above shown reflection to the (private) base methods for rendering, so i am not sure how your legal people feel about that one. It does add significant functionality to RadPane, which people might like.

The main reason i suggested the RadScriptedControl and RadSkinnedControl (albeit that the latter is easy to implement), is that RadWebControl sticks that hidden ClientState input into any control made with it, whether it is needed or not. In the case of, say, our custom RadGroupBox control, we do not need ClientState, in fact, it only needs a little skin info (using the above CSS extraction code) and scripting support (for sizing, header or no header, etc). So, that means that deriving it from RadWebControl inserts a hidden input (that automatically gets populated with information) while we do not require it (however, if the group box supports scrolling, then it is handy to have that ClientState input, but we design our UIs to be mostly non-scrolling, big exception being the RadHeaderPane, which uses your scrolling implementation for RadPane in the 'inner content panel' of the header (when used)).

So instead of another layer of inheritance, it is more the inclusion of a few more base controls than anything else. Your script referencing is significantly different from 'usual' and requires some reflection to your ScriptRegistrar. It would be great to not have to resort to so much reflection (in total) in order to get a simple control like a group box (with or without header) developed. This is besides the ClientState thing.

Quirks-mode has always kind of been a mystery to me, even with the massive amount of customized controls, some REALLY strange work-arounds, code generators, page generators, etc inside our framework, i've never actually had quirks-mode 'active'. As a matter of fact, i don't think i have ever SEEN it active in the first place, my developers pretty much shrug it off as 'who cares' as well. Would you mind elaborating a little on the circumstances that quirks-mode would be active on, and what it means for an AJAX based web application in general? (It is not an issue with our framework, but i am in the process of updating a few of our previous products for customers, so, said information could be useful in the way of steering clear of quirks-mode.)

I am glad you agree on the 'private' methods needing to be 'protected' (or even 'public' in some places), it's been a bit of a pain through-out our development process regarding the framework. On the script and skin registrars, the biggest one for us would be the EmbeddedSkinAttribute, since reflection on SkinRegistrar is not an issue but the resulting error messages regarding the skins due to the lack of access to this attribute is a bit of an issue (and a fairly nasty work-around to fix).

Have you guys had a chance to review my "Yet Another RadMenu Client-Side Script Issue" post yet? It exposes a rather significant amount of client-side issues with RadMenu (excluding the one that started this thread), as well as the fixes i applied for them.

Once more, thanks for your time, i highly appreciate the level of information you are willing to give, as well as a degree of insight on the how and why of certain Telerik things.

Regards,

Mike

[T. Tsonev]

Hello,

The CSS extraction code might prove useful for others and it's kind of you to share it. Please, feel free to submit it as a Code library. Even though most of it doesn't deal with Telerik controls directly, it's a lightweight CSS parser that anyone can use for his own ideas. Using full-weight tools like ANTLR or GOLD might still be necessary for some, but might be an overkill for smaller projects.

We'll take the RadHeaderPane as a suggestion for functionality that we can add to our Pane.

The ClientState field can be easily disabled by overriding the RenderClientStateField and LoadPostData methods with empty methods.

Quirks mode doesn't mean much to most application developers. You just put a DOCTYPE on top of your page and you're done with it. Unless you support IE 5.5, you'll never hear of it again.

But quirks mode means a lot more to control developers, as we don't have control over the page that the controls will be placed on we have to support quirks mode as well. And that's not rare, SharePoint and older versions of DNN render in quirks mode by default.

More info about standards vs. quirks:
http://www.quirksmode.org/css/quirksmode.html

Kind regards,
Tsvetomir Tsonev
the Telerik team

Instantly find answers to your questions on the new Telerik Support Portal.
Check out the tips for optimizing your support resource searches.