Fluent:RibbonAttachedProperties.RibbonSizeDefinition

Aug 1, 2013 at 5:19 PM
I can understand improving architecture by making dependency properties attached, but making users write Fluent:RibbonAttachedProperties.RibbonSizeDefinition="Middle" instead of SizeDefinition="Middle" is plain inhuman. :) This is one of the most used properties.
Developer
Aug 1, 2013 at 6:31 PM
So, calling my change inhuman is way too much for me to stay calm.
This is one of the most used properties
I really would like to know how you gathered this information. How many users did you ask? Or are you just saying it because you use it often?

First:
If writing 40 characters more, or even copying them (you are aware of copy and paste?!?), is inhuman, create a fork, change it the way you like and send a pull request. I won't revert that change.

Second:
Go ahead and talk to the people requesting full binding capabilities.

Third:
Go ahead and talk to the people trying to use third party or custom controls without the need to subclass all of them and implement IRibbonControl.

This library is not only free, it's open source and i am spending my spare time to contribute to it, so calling anything i change inhuman just because you have to write 40 characters more in, let me guess, 3.5 billion places in your code, is not only unfriendly but offending and unthankfull in addition. In addition to that, adding a smiley makes it even worse.
Aug 1, 2013 at 7:29 PM
I'm bad at jokes, but you're even worse at getting them... Didn't mean to offend, sorry.

Seriously, no need to sacrifice XAML readability. XAML is verbose enough already.

No need to call something RibbonAttachedProperties, then call a property inside it RibbonSizeDefinition - the word "ribbon" is redundant here.

No need to give a class such a long name. No need to use a separate class either. Grid.Column is not named GridAttachedProperties.GridColumn. TextElement.FontFamily is not TextElementAttachedProperties.TextElementFontFamily. The only case I know of a separate class from .NET Framework solely for attached properties is AutomationProperties, but even it is not named AutomationAttachedProperties.

Moving Size, SizeDefinition (and maybe other) properties to Ribbon class may be a good idea. It's not perfect from single responsibility principle's point of view, but code in RibbonAttachedProperties is mostly trivial, so it doesn't matter much.

This is no way will hinder people who want full binding capabilities (whatever that means) or want to avoid subclassing controls (which is a great improvement, I agree). The current sad reality is, most XAML is written by hand, therefore it is important to keep it simple and readable.

As to the popularity of properties, you can look at TestWindow.xaml and count usages. (While I'm here... Adding TextOptions.TextFormattingMode="Display" SnapsToDevicePixels="True" to the RibbonWindow will improve how it looks.)

P.S. The library is awesome. It's way better than Microsoft's half-baked ribbon riddled with bugs. This is why I care about the way the library evolves. If I didn't, I'd just create yet another "shortener" FluentProps and be happy with it.
Jun 1, 2014 at 1:29 PM
So, if I move and rename the attached properties the way I suggested and make a pull request, will you accept it? Or do you have any objections?
 
  • RibbonAttachedProperties.TitleBarHeight -> Ribbon.TitleBarHeight
  • RibbonAttachedProperties.RibbonSize -> Ribbon.Size
  • RibbonAttachedProperties.RibbonSizeDefinition -> Ribbon.SizeDefinition
Not sure about the proper place for GetThreeRibbonSizeDefinition and SetAppropriateSize. Can probably be moved to Ribbon too.

There's also another approach (commonly used in WPF itself) — use DependencyProperty.AddOwner and add getters and setters to all bundled controls. This has the benefit of backwards compatibility and not disturbing your naming conventions, but those who put custom controls into ribbons will still suffer from verbosity if they don't subclass.
Developer
Jun 18, 2014 at 2:17 PM
Adding setters and getters to all controls won't be a practical solution.

What about renaming it to RibbonProperties?

Ribbon.TitleBarHeight kind of belongs more directly to the Ribbon, so that would be ok.
Even though it technically belongs or describes a property of the window the ribbon is hosted in. But moving that property to the RibbonWindow would be wrong because we also got the MetroWindow.

But Ribbon.Size and Ribbon.SizeDefinition define the Size and SizeDefinition of a control contained in the Ribbon and not the Size and SizeDefinition of the Ribbon itself, so it might confuse people if those properties are found directly in Ribbon.

It's, sadly, not that easy do decide where to put all these things...

Do you get what i mean? English is not my native language, so please tell me if anything seems unclear in my description.
Jun 18, 2014 at 3:15 PM
Why do you think that adding getters and setters to all controls is impractical? This is what is done in .NET framework itself. AddOwner is used heavily. Not sure how to give links to reference sources, it doesn't work well... Open these links, then click on highlighted "AddOwner" in the sources to see all usages:
http://referencesource.microsoft.com/WindowsBase/R/6e23361031c54c10.html
http://referencesource.microsoft.com/WindowsBase/R/e9e7035f1e4436db.html
If you look at RoutedEvent.AddOwner you'll find even more usages:
http://referencesource.microsoft.com/PresentationCore/R/cdce3fd8b4cc817b.html
There're hundreds of AddOwner calls (around 400-500 overall), all are accompanied with getters and setters (looks like), so Microsoft's designers think this approach is fine.

"RibbonProperties" is an improvement, but it's a half measure.

Attached properties aren't supposed to describe the controls they're (in C# sense) contained in. Grid's attached properties Column, Row, ColumnSpan, RowSpan describe the control they're attached to, not the grid. This concept isn't alien to WPF, so it shouldn't be confusing. Actually, Size and SizeDefinition are somewhat similar in functionality to attached properties of WPF container controls as they define how a control is presented in the ribbon.

Overall, I think AddOwner is the way to go, no matter in what class the properties are contained (Ribbon or RibbonProperties). Looks like it's the "WPF way" of handling similar properties in different controls. This solution also has the benefit of preserving backward compatibility with 2.0 version, besides being the most concise and preserving all new and old functionality (if someone can't or doesn't want to subclass his controls, he can still use full attached property syntax).

English isn't my native language either, so don't worry.
Developer
Jun 20, 2014 at 11:52 AM
Edited Jun 20, 2014 at 11:52 AM
I don't get your argument.

You talk about using AddOwner and adding getters and setters on one side.
On the other side you talk about Grid.Row etc. which is not used that way.

Even looking at Grid and the way Row, Column etc. are handled would mean to not use Ribbon.Size, because that's not the container the controls are placed in.

I would agree, after having a look at Grid, to rename/move the properties to Ribbon to have Ribbon.Size.
But i won't agree to use AddOwner, as i don't see any benefit in doing so.
Jun 20, 2014 at 2:07 PM
Logic for using AddOwner is simple: IF different controls have the same property/event AND class hierarchy can't be used THEN declare the property/event in one control (or a separate class), share with other controls using AddOwner. Simple as that. There're no additional requirements.

The same with attached properties in general. There're no requirements that attached properties declared in a control are used only for layout, or can be attached only to direct children in logical tree, or must affect only layout, or can be used just for containers. Grid.Row is just an example how attached properties can be used, but WPF in no way limits that.

Have you looked at the usages from the links I've provided?

ContextMenuService.PlacementTargetProperty, PlacementRectangleProperty, PlacementProperty are shared with ContextMenu (properties control owner window placement, declared in a separate class).
ToolTipService.HorizontalOffsetProperty and others are shared with ToolTip (similar to above).
Stylus.IsPressAndHoldEnabledProperty, IsFlicksEnabledProperty and others are shared with FrameworkContentElement and FrameworkElement (properties control behavior — responding to stylus events).
ScrollViewer.HorizontalScrollBarVisibilityProperty, VerticalScrollBarVisibilityProperty are shared with various controls having scrollbars (properties control visibility of template parts, declared in a control).
Selector.IsSelectedProperty is shared with ListBoxItem and other items (property controls behavior and style, declared in a container).

Attached properties control templates, styles, behavior, layout — everything. They're designed for this.

What is your reason NOT to use AddOwner?
Developer
Jun 24, 2014 at 9:29 PM
I agree that we should rename/move those properties.

The reasons why, at least, i won't add AddOwner (+ getters and setters) to all the bundled controls are:
  • I don't need it, as everything is working fine, and my time is very limited
  • It's not necessary (in terms of a working library)
  • It's "just" a shortcut
To be honest, please stop trying to teach me about WPF in the way you are trying to do it.
Do you really think that i don't know what attached properties etc. are and how they can be used?

Besides, writing in caps is like screaming at someone.
Jun 24, 2014 at 11:58 PM
If your time is limited, I can do it myself. It's time well spent because it'll save time when porting apps to 2.1 from previous versions of the library. I'd better spend my time once on the library than make all users of the library, including myself, spend time on every app instead. I also prefer typing XAML manually, so amount of typing matters to me.

I don't get your point of the feature being "unncessary". There're a lot of "shortcuts" in C#, .NET, WPF. You don't set placement on context menus using "ContextMenuService.Placement", do you?

Does the feature I propose has any real disadvantages you want to avoid? If it's just a matter of saving your time, then I'll add the properties myself and create a pull request. I just need your approval before doing anything.

The reason I've provided examples of usages is that you've said previously: "Even looking at Grid and the way Row, Column etc. are handled would mean to not use Ribbon.Size, because that's not the container the controls are placed in." To me, it looked like you wanted to limit capabilities of attached properties.

P.S. Using caps on single words is a pretty common way of emphasizing. I could've used italics or bold, but I was lazy. Also, italics and bold aren't always available, so caps are often the only way to emphasize words. Using quotation marks for this is incorrect, by the way (it would look like irony, i.e. meaning is inverted).
Developer
Jun 25, 2014 at 5:40 PM
I said:
•It's not necessary (in terms of a working library)

If you got the time, feel free to implement the changes and create a pull request.
Jun 25, 2014 at 8:30 PM
Okay, awesome.

And the last question: where would you like the following properties to be placed and how should they be named?
  • TitleBarHeight
  • (Ribbon)Size
  • (Ribbon)SizeDefinition
Options:
  1. Class:
      a. Keep in RibbonAttachedProperties
      b. Rename RibbonAttachedProperties to RibbonProperties
      c. Move to Ribbon
  2. Name:
      a. Keep Ribbon prefix
      b. Remove Ribbon prefix
I'd prefer 1.c+2.b for all three properties, but it doesn't matter to me much (as long as I have shortcuts), so I'll just implement whatever you choose. From the discussion above, your choice for the three properties seems to be 1.c, 1.b, 1.b (accordingly), with 2 being unclear, so I want to be sure.
Developer
Jun 25, 2014 at 9:18 PM
  • Move to RibbonControl
  • Remove Ribbon prefix
That seems to be the best solution, doesn't it?

And you should implement your changes (AddOwner, Getter, Setter) in RibbonControl first, that way Backstage, RibbonToolBar, Spinner and TextBox automatically inherit them.
Jul 9, 2014 at 6:24 PM
I don't think the same property can be both attached and not attached in a class at the same time (haven't actually checked, but it looks incredibly weird to have both property and methods accessors), so I haven't moved the properties to RibbonControl and just renamed everything instead (removed "Attached" class infix and "Ribbon" property prefix) and added properties to all classes from which they were removed (by the "BREAKING CHANGES!!!" commit) and which implement IRibbonControl interface.

Attached properties can be moved from RibbonProperties to Ribbon, if you think it's acceptable.

I'm not sure whether some classes need Size and SizeDefinition properties: ColorGallery, Gallery, GalleryGroupContainer, GalleryPanel, RibbonToolBarControlGroup, RibbonToolBarControlGroupDefinition. I'm also not sure what classes need TitleBarHeight property. It doesn't seem to be used heavily (outside of internal styles), so I don't think it really matters. However, from the discoverability point of view adding shortcut properties may be a good idea.

There's another attached property, KeyTip.Keys. It's heavily used too, so I've added KeyTip property to all controls which implement IKeyTipedControl interface.

Also, is there any reason for SizeDefinition property to be a string? Why not use a structure with a type/value converter? Parsing will be avoided on every resize and, well, strongly typed properties are always better than not strongly typed. :)

Pull request: https://fluent.codeplex.com/SourceControl/network/forks/Athari/Fluent/contribution/7095
Developer
Jul 9, 2014 at 7:40 PM
Also, is there any reason for SizeDefinition property to be a string? Why not use a structure with a type/value converter? Parsing will be avoided on every resize and, well, strongly typed properties are always better than not strongly typed. :)
Will have a look at that.
Developer
Jul 9, 2014 at 9:24 PM
Will change SizeDefinition from string to IList<RibbonControlSize> and use a TypeConverter.

Any complaints or suggestions?
Jul 9, 2014 at 10:35 PM
I think a custom structure would be preferable.
  1. You can't attach TypeConverterAttribute to IList, so every property (one attached and all shortcuts) will have to be decorated with the attribute instead of just one type. This complexity also expands to users of the library, if they want to subclass controls and add Size & SizeDefinition properties — they will need to remember to put the attribute.
  2. The interface of IList is too broad — you don't want users to add or remove elements from it, yet the interface suggests that they can.
  3. IList interface doesn't tell how the values are used (as Large, Middle, Small).
  4. Making dependency properties collections is asking for trouble. There's no sensible way to define a default value for them (default value will be shared), owner is not automatically notified about item changes etc. The correct way to make a collection property is to make a reado-only property returning observable collection, create the collection in constructor, subscribe to its changes etc., which sounds as too much trouble for something so simple.
My suggestion is to create a structure with three properties of type RibbonControlSize: Large, Middle, Small. System.Windows.Thickness class can be used as an example of a very similar type.
Developer
Jul 10, 2014 at 12:03 AM
Edited Jul 10, 2014 at 12:03 AM
  1. Not true. Adding the converter to the static getter is enough.
  2. Your point.
  3. Your point, but it has to be well documented in the new class that those properties (Large, Middle, Small) correspond to the GroupBoxState (to finally make it clear for everyone).
  4. Null would be valid here to avoid sharing. The suggested structure wouldn't notify about changes too, so i don't get that point.
Suggestion:
  • New immutable class RibbonSizeDefinition
  • Properties of that class: Large, Middle, Small
  • TypeConverter on class to ease use in xaml
  • Implicit cast operators (from and to string) to not break existing code
Jul 10, 2014 at 4:46 AM
Not true. Adding the converter to the static getter is enough.
Tested. You seem to be right, WPF is smarter than I thought. :)
Null would be valid here to avoid sharing.
Default nulls will cause issues when a collection is filled as a collection from XAML. XAML won't instantiate a collection and NRE will be thrown.
The suggested structure wouldn't notify about changes too, so i don't get that point.
You can't get a reference to a structure from a dependency property — you'll always get a copy as it's a value type. Setters on struct properties are useful only during initialization, using them to change the struct's value will cause a compilation error. Setting Margin.Bottom = 1 would fail, even though Thickness.Bottom has a setter.
New immutable class RibbonSizeDefinition
Both a structure and immutable class would work, I guess. I think a structure would be better, because sematically it's a value type. If you don't have strong reasons to use a class, I think doing like in the framework is a good idea.
Implicit cast operators (from and to string) to not break existing code
Implicit operators defeat the purpose of strongly typed properties. They move value validation from compile time to run time.

It's against Microsoft's design guidelines ("Do not provide a conversion operator if such conversion is not clearly expected by the end users") as conversion to and from string isn't something obvious. .NET developers expect strings from ToString method which is designed for this. Even StringBuilder doesn't have an implicit cast operator.

Implicit operators can sort all sorts of problems (ambiguity, information loss, issues with equality etc.), so it's usually advised to avoid them.

The new version will break backward compatibility anyway, right? And I don't think this property is often assigned from the code... Ribbon design configuration should be static.

By the way, to avoid verbosity in C# code, I propose adding all sensible combinations as static properties:
  • SmallSmallSmall
  • MiddleSmallSmall
  • MiddleMiddleSmall
  • MiddleMiddleMiddle
  • LargeSmallSmall
  • LargeMiddleSmall
  • LargeMiddleMiddle
  • LargeLargeSmall
  • LargeLargeMiddle
  • LargeLargeLarge
I'd rather write RibbonSizeDefinition.LargeMiddleSmall than new RibbonSizeDefinition(RibbonControlSize.Large, RibbonControlSize.Middle, RibbonControlSize.Small).

And regarding type name: I think naming types RibbonControlSize and RibbonSizeDefinition is inconsistent. They should either be RibbonControlSize and RibbonControlSizeDefinition, or RibbonSize and RibbonSizeDefinition.
Jul 10, 2014 at 4:59 AM
Huh. Now that I've listed all values, it sounds like a enum for me. %) If the above list is correct, then the following enum includes all sensible values and shortcuts:
    public enum RibbonControlSizeDefinition
    {
        SmallSmallSmall,
        MiddleSmallSmall,
        MiddleMiddleSmall,
        MiddleMiddleMiddle,
        LargeSmallSmall,
        LargeMiddleSmall,
        LargeMiddleMiddle,
        LargeLargeSmall,
        LargeLargeMiddle,
        LargeLargeLarge,

        //SmallSmall = SmallSmallSmall,
        MiddleSmall = MiddleSmallSmall,
        //MiddleMiddle = MiddleMiddleMiddle,
        LargeSmall = LargeSmallSmall,
        LargeMiddle = LargeMiddleMiddle,
        //LargeLarge = LargeLargeLarge,

        Small = SmallSmallSmall,
        Middle = MiddleMiddleMiddle,
        Large = LargeLargeLarge,
    }
Have I missed anything?
Developer
Jul 10, 2014 at 7:30 PM
Edited Jul 10, 2014 at 7:30 PM
Implicit operators defeat the purpose of strongly typed properties. They move value validation from compile time to run time.
But in XAML it's ok?
By adding an implicit cast operator we won't break any code which assigns values in code or reads them back.
So i think it's kind of expected by end users that we don't break their code. We are already breaking more than enough code with the next release.
And I don't think this property is often assigned from the code...
Our applications never assign it in XAML but in code instead. All the time. Because the library still does not support MVVM enough to a useable that way.
Ribbon design configuration should be static.
We don't use XAML to compose the Ribbon in our applications.
Both a structure and immutable class would work, I guess. I think a structure would be better, because sematically it's a value type. If you don't have strong reasons to use a class, I think doing like in the framework is a good idea.
Default values (Large, Middle, Small)? Not possible with struct.
But we can try. Defining one instance with default values as default value for the attached property should work.
RibbonControlSizeDefinition
Good idea. Sounds better than RibbonSizeDefinition.

Regarding the enum idea:
How to do the mapping from GroupBoxState to RibbonControlSize?
Gigantic switch?
Jul 11, 2014 at 6:55 AM
But in XAML it's ok?
XAML isn't strongly typed (some parts are, but not enough). And it often sucks. C#, on the other hand, is strongly typed, so adding implicit operators removes the benefit.
So i think it's kind of expected by end users that we don't break their code.
The problem is, this behavior is only expected by previous users. New users will be surprised.

The question is, how hard will it be to fix the code? If a user relies on string constants like "Large, Middle, Small", it's a trivial fix (try compiling, get a list of errors, then just replace every string with SizeDefinition.Whatever). If a function generates these values, then it needs to be changed, but it should be simple, considering strings and values are easily mapped (Enum.Parse/ToString). Overall, it shouldn't be hard to fix in a well-structured code, I think.
Our applications never assign it in XAML but in code instead. All the time. Because the library still does not support MVVM enough to a useable that way.
Why? Are your applications extensible with plugins which add ribbon controls? Or do you actually change controls every time based on application state?

I mean, let's look at MS Word. Addins and user can customize the ribbon somewhat, but almost all controls never change. Tabs, groups, buttons are always the same. Contextual tabs appear and disappear, controls are enabled and disabled, but you never see groups or buttons being added, rearranged etc. Ribbon's layout is static and there's a reason fot this — its perfectly optimized.

So I wonder why XAML, which allows similar design capabilities, isn't enough.
But we can try. Defining one instance with default values as default value for the attached property should work.
By the way, it occured to me why properties of Thickness have setters. It allows element syntax:
    <Control.Margin>
        <Thickness Left="1" Top="2" Right="3" Bottom="4"/>
    </Control.Margin>
How to do the mapping from GroupBoxState to RibbonControlSize?
Switch won't be large, there're only ten possible control size definitions. :)

Another possibility is bit twiddling. Not sure about RibbonGroupBoxState mapping (I don't see it in the code), but RibbonControlSize can be extracted from RibbonControlSizeDefinition (and vice versa) like this:
    internal class Program
    {
        private static void Main ()
        {
            RibbonControlSizeDefinition def = new[] { RibbonControlSize.Large, RibbonControlSize.Middle, RibbonControlSize.Small }.ToSizeDefinition();
            WriteSizeDefinition(def.ToString(), def);

            def = "Middle > Small".ToSizeDefinition();
            WriteSizeDefinition(def.ToString(), def);

            def = RibbonControlSizeDefinition.LargeSmall;
            WriteSizeDefinition(def.ToString(), def);

            Console.WriteLine("All values:");
            foreach (string defName in Enum.GetNames(typeof(RibbonControlSizeDefinition)))
                WriteSizeDefinition(defName, (RibbonControlSizeDefinition)Enum.Parse(typeof(RibbonControlSizeDefinition), defName));
            Console.ReadKey();
        }

        private static void WriteSizeDefinition (string defName, RibbonControlSizeDefinition def)
        {
            Console.WriteLine("{0} = {1} {2} {3}", defName,
                def.GetSize(RibbonControlSize.Large), def.GetSize(RibbonControlSize.Middle), def.GetSize(RibbonControlSize.Small));
        }
    }

    public enum RibbonControlSize
    {
        Small = 2,
        Middle = 1,
        Large = 0,
    }

    public enum RibbonControlSizeDefinition
    {
        SmallSmallSmall = RibbonControlSize.Small << 0 | RibbonControlSize.Small << 2 | RibbonControlSize.Small << 4,
        MiddleSmallSmall = RibbonControlSize.Middle << 0 | RibbonControlSize.Small << 2 | RibbonControlSize.Small << 4,
        MiddleMiddleSmall = RibbonControlSize.Middle << 0 | RibbonControlSize.Middle << 2 | RibbonControlSize.Small << 4,
        MiddleMiddleMiddle = RibbonControlSize.Middle << 0 | RibbonControlSize.Middle << 2 | RibbonControlSize.Middle << 4,
        LargeSmallSmall = RibbonControlSize.Large << 0 | RibbonControlSize.Small << 2 | RibbonControlSize.Small << 4,
        LargeMiddleSmall = RibbonControlSize.Large << 0 | RibbonControlSize.Middle << 2 | RibbonControlSize.Small << 4,
        LargeMiddleMiddle = RibbonControlSize.Large << 0 | RibbonControlSize.Middle << 2 | RibbonControlSize.Middle << 4,
        LargeLargeSmall = RibbonControlSize.Large << 0 | RibbonControlSize.Large << 2 | RibbonControlSize.Small << 4,
        LargeLargeMiddle = RibbonControlSize.Large << 0 | RibbonControlSize.Large << 2 | RibbonControlSize.Middle << 4,
        LargeLargeLarge = RibbonControlSize.Large << 0 | RibbonControlSize.Large << 2 | RibbonControlSize.Large << 4,

        //SmallSmall = SmallSmallSmall,
        MiddleSmall = MiddleSmallSmall,
        //MiddleMiddle = MiddleMiddleMiddle,
        LargeSmall = LargeSmallSmall,
        LargeMiddle = LargeMiddleMiddle,
        //LargeLarge = LargeLargeLarge,

        Small = SmallSmallSmall,
        Middle = MiddleMiddleMiddle,
        Large = LargeLargeLarge,
    }

    internal static class RibbonControlSizeExts
    {
        public static RibbonControlSize GetSize (this RibbonControlSizeDefinition @this, RibbonControlSize index)
        {
            return (RibbonControlSize)((int)@this >> (int)index * 2 & 3);
        }

        public static RibbonControlSizeDefinition ToSizeDefinition (RibbonControlSize large, RibbonControlSize middle, RibbonControlSize small)
        {
            return (RibbonControlSizeDefinition)((int)large << 0 | (int)middle << 2 | (int)small << 4);
        }

        public static RibbonControlSizeDefinition ToSizeDefinition (this IList<RibbonControlSize> sizes)
        {
            switch (sizes.Count) {
                case 0:
                    return RibbonControlSizeDefinition.Large;
                case 1:
                    return ToSizeDefinition(sizes[0], sizes[0], sizes[0]);
                case 2:
                    return ToSizeDefinition(sizes[0], sizes[1], sizes[1]);
                default:
                    return ToSizeDefinition(sizes[0], sizes[1], sizes[2]);
            }
        }

        public static RibbonControlSizeDefinition ToSizeDefinition (this string strSizes)
        {
            if (string.IsNullOrWhiteSpace(strSizes))
                return RibbonControlSizeDefinition.Large;
            return strSizes.Split(new[] { ' ', ',', ';', '-', '>' }, StringSplitOptions.RemoveEmptyEntries)
                .Select(s => (RibbonControlSize)Enum.Parse(typeof(RibbonControlSize), s, true))
                .ToList()
                .ToSizeDefinition();
        }
    }
All bit logic is within the first two methods of RibbonControlSizeExts. I've also set Large to 0, so default value will be LargeLargeLarge.
Developer
Jul 14, 2014 at 6:45 PM
XAML isn't strongly typed (some parts are, but not enough). And it often sucks. C#, on the other hand, is strongly typed, so adding implicit operators removes the benefit.
It does not remove the benefit. It weakens it. But i can't see a reason for not adding it.
The problem is, this behavior is only expected by previous users. New users will be surprised.
Again, we introduced enough breaking changes. New users would have to try to assign a string to be surprised.
It's against Microsoft's design guidelines ("Do not provide a conversion operator if such conversion is not clearly expected by the end users") as conversion to and from string isn't something obvious.
Please read those guidelines fully and don't just paste what you like most. The guidelines state "Ideally, customer research data should exist to support defining a conversion operator." in the next line. We got research data, our current users.
Why?
Because we use it that way.
Are your applications extensible with plugins which add ribbon controls?
Yes. Everything is treated as a plugin.
Or do you actually change controls every time based on application state?
Not on state but on context and those menus are composed of default items and individual items.
Not sure about RibbonGroupBoxState mapping (I don't see it in the code),
         public static void SetAppropriateSize(DependencyObject element, RibbonGroupBoxState state)
         {
            var index = (int)state;
            if (state == RibbonGroupBoxState.Collapsed)
            {
                index = 0;
            }

            SetSize(element, GetThreeRibbonSizeDefinition(element)[index]);
        }
So i still think, going the Margin/Thickness way is superior to an enum.
Judging from my experience, most people don't get bit logic as easy as they should.
Jul 17, 2014 at 7:21 AM
Again, we introduced enough breaking changes.
Well, if you think it's that crucial, you can mark the operators as [Obsolete]. The compiler will warn about all usages, so the user will be able to notice they can change their code to use strongly typed values.

However, I still think that conversion to string should be explicit, even if there's an implicit conversion from string. Implicit operator changes behavior of formatting functions (object vs string argument: Console.WriteLine(foo.SizeDefinition)) and concatenation, it doesn't convert the value when accessed using "." (foo.SizeDefinition.Split(' ')). So it's useless at best.
 
So i still think, going the Margin/Thickness way is superior to an enum.
What are the real benefits? The only one I see is having an implicit cast operator from string.

The code above becomes something like this:
RibbonControlSize index = RibbonControlSize.Large;
if (state <= RibbonGroupBoxState.Small)
  index = (RibbonControlSize)state;
SetSize(element, GetSizeDefinition(element).GetSize(index));
By the way, there's one more benefit of having enums — you get code completion in XAML for free (need to check this though, I'm not entirely sure how the editor handles enums with converters).
Judging from my experience, most people don't get bit logic as easy as they should.
Conversion methods should be provided by the library, of course. Bit logic like this isn't something that should be done in the user's code, it's too easy to mess it up.
Jul 20, 2014 at 10:28 PM
Just changed my references to use your beta code.
Found a breaking change for me....

Getting null reference errors because for my code element in SetAppropiateSize is null.

I assume this is coming about because I am dynamically adding items to the ribbon that has not been drawn yet?

this.ItemContainerGenerator.ContainerFromItem(item) return null, item is a Fluent button.

All was working well prior.
Developer
Jul 21, 2014 at 10:29 PM
This discussion has been copied to a work item. Click here to go to the work item and continue the discussion.
Developer
Jul 21, 2014 at 10:45 PM
However, I still think that conversion to string should be explicit, even if there's an implicit conversion from string. Implicit operator changes behavior of formatting functions
It doesn't. It does the same ToString does.
Conversion methods should be provided by the library, of course. Bit logic like this isn't something that should be done in the user's code, it's too easy to mess it up.
But others might want to read our code and contribute to it.
By the way, there's one more benefit of having enums — you get code completion in XAML for free (need to check this though, I'm not entirely sure how the editor handles enums with converters).
You also get code completion in XAML when you use the property setter version of my solution instead of the string shortcut way.


To bring this discussion to an end, i will simply implement my solution.