Added 'nil' display value to optionally nullable values in the table view

[TimOliver]

Up until now, the Browser didn't really properly represent item values that were 'truly' nil. String values were rendered as empty test fields, and numeric values were being displayed as '0'.

This PR introduces the ability for the Browser to properly differentiate nil values, with a red 'nil' string being displayed in property types that are both optional, and presently empty.

Let me know what you think of that visual style, and if you think there's something more appropriate we should use. :)

/cc @jpsim @stel

[stel]

I think using lower case for True and False should be more convenient

[stel]

I have some suggestions here :)

• Red color might be treated as error, but nil value is a valid value for optional properties.
• Is it possible to use RLMOptionalBoolTableCellView (or implement this in RLMBoolTableCellView) to display non-optional properties also? Currently there is a different display style for optional bool (PopUp button) and non-optional (Checkbox). Also this should allow to use BoolCell the same way as any other cells and there will be only one type of bool cells :)
• Just a question: does an empty string mean nil for String properties? Or how could a nil value be specified in text field?

[jpsim]

Red color might be treated as error, but nil value is a valid value for optional properties.

I'm not opposed to using red, but if we're looking for alternatives, maybe just light gray to appear dimmed out is better?

[TimOliver]

Fair enough! Thanks for the feedback guys!

I tried changing nil to light grey instead, and that definitely looks a bit nicer. What do you think?

That being said, when you set the colour via NSAttributedString, I've noticed they've also stopped highlighting white when you select the row. @stel, do you know if there's a quick way to fix that?

In response to the other queries:

Is it possible to use RLMOptionalBoolTableCellView (or implement this in RLMBoolTableCellView) to display non-optional properties also?

I was considering just extending the RLMBoolTableCellView class, but since it needs a whole new view to manage (i.e. NSPopUpButton as opposed to the checkbox), and there's no guarantee of both an optional and a non-optional bool both being in one Realm table, it seemed cleaner to make them two separate cells.

Just a question: does an empty string mean nil for String properties? Or how could a nil value be specified in text field?

At the moment, if the string supports nullability, and you delete everything in there, it'll defer back to nil. If the string was explicitly marked as non-optional, then the Browser will automatically fill it with an empty string (i.e. @""). I was wondering if there might be an instance where you have an optional string, and would actually like to populate it with an empty, non-nil string value, but I'm not sure if there's a way we could represent that in the Browser.

[stel]

Light grey looks good! As for highlighting the only thing that comes to mind is to override - setBackgroundStyle: in NSTableCellView subclasses and set the white color for placeholder string when cell is selected. You can handle this by NSBackgroundStyleDark or NSBackgroundStyleLight.

As for RLMBoolTableCellView maybe it's better to use NSPopUpButton instead of checkbox for non-optional values too (without nil option in popup menu) to unify a display of both Bool values?

[astigsen]

if the string supports nullability, and you delete everything in there, it'll defer back to nil. If the string was explicitly marked as non-optional, then the Browser will automatically fill it with an empty string (i.e. @""). I was wondering if there might be an instance where you have an optional string, and would actually like to populate it with an empty, non-nil string value

The browser should definitely support differentiating between nil and the empty string (both visually and when editing). I would suggest that deleting the string makes it an empty string, and then offering the option to set it to nil when you right-click it.

[TimOliver]

@stel: Awesome! Thanks for the tip. I was able to get all of the elements coloured properly when selected now:

(Apparently it's not possible to change the colour of the 'handle' next to the optional BOOL out of the box, but apparently there's a library called ButterKit that can add this functionality. I'll check this one out more down the line.)

As for RLMBoolTableCellView maybe it's better to use NSPopUpButton instead of checkbox for non-optional values too (without nil option in popup menu) to unify a display of both Bool values?

I don't know about that. When a BOOL isn't optional, from a user experience perspective, it seems to make more sense/is also slightly faster to use a proper checkbox to denote that there are only 2 states.

I still think it makes sense to keep these two cell types separate due to the way they both behave differently. Maybe @jpsim can chime in with his opinion on this. :)

---

@astigsen Okay! Fair enough. There could easily be use cases where an app could be using nil and @"" to differentiate its logic (Although I'm not sure if that's always best practice!). If a string is optional, I think it makes more sense to assume that the string is nil when completely empty, and @"" should be treated as a special case.

I've added an extra menu item named "Insert empty string value" that appears when you right-click on an optional string column. I'm still working out how to display this difference visually in the Browser (It turns out NSTextField cannot accept nil strings, and must accept @"" by default, so our architecture of just checking the text field's value isn't sufficient), so I'll follow that up later tonight.

[bdash]

Treating the empty string as nil if the property is nullable seems incredibly confusing. We'd clearly still be treating it as an empty string if the property is not nullable. That means you have to do different things to get the same value for the property depending on whether it's nullable or not.

[TimOliver]

Hopefully it just boils down to the point that we represent the property in the UI properly. If you completely clear a string property that was marked nullable, then it'll display nil in the text field, which (assuming the user knew they made it nullable) should make intuitive sense.

If the string property was not marked optional, then by default, when clearing the text field, it should be deferred to an empty, non-null string.

In the case that the user wants to make a nullable string filled, but not hold a value, we can use an empty string value. I'm going to see if I can make the text field display <empty> instead of nil in that case.

Again, I don't think there'll be too many use cases where there'll be a nullable string field, but the user will want to insert a non-nil, yet empty string. But if everyone would prefer I do it that way, I'll defer to your judgment. :)

[TimOliver]

Actually, I've played with it for a while now, and it turns out that trying to keep a proper separation between nil and @"" states in the Browser code is pretty tricky. I think I've almost got it, but I'm getting some weird behaviour (Mainly apparently caused by an enumeration getting mutated somewhere in the code) that is causing this to break.

I'll look at it again next week, but I might need to rethink how this is being handled and displayed in the Browser.

[stel]

I like the idea with popup menu and empty placeholder 👍

From one hand it's not straightforward to set optional string to nil when erasing the content of textfield (because the same action will set empty for non-optional string), but from another hand, more users will expect nil in this case.

[TimOliver]

Alrighty. I need to push this along since I've got some other issues to start. :)

I discovered that glitchy enumeration bug was being cause by my code trying to change the placeholder text colour while it wasn't on the main thread. So I've fixed that, and it's all working perfectly now.

Since it's holding up actually having nullability at all in the Browser right now, I actually think the @"" vs nil strings might be better off in its own issue that we can discuss properly and integrate later. So I've reverted those commits, and will move the lot to a new issue.

As for the checkbox vs popup debate, I still think this is the best solution from a user experience solution. While we should definitely aim to compact the number of custom cells we have, I think in this case, it makes sense to have two.