Fix problem with implicitly defined initial direction

[semion1956]

Issue #381

[wkeese]

I think I've said a few times before that effectiveDir should replace isLeftToRight(). That means that effectiveDir should take the value or dir when dir is set (and that should be handled in computeProperties().

Also, as I've said before, all features, including this one, need automated tests.

Thanks.

[semion1956]

Yes, you said (BTW, it was in a slightly different contexts), but that is what I had in mind: 1) isLeftToRight() is known public method, looks simply and sounds natively. effectiveDir is something internal and (in my opinion) user should not be aware of it. 2) if some widget has explicitly defined dir and somebody changes its value to empty, we need recompute value of effectiveDir once again - in our case, using getComputedStyle(), but it is what you require to avoid. Do you think, that both these reasons aren't important?

[wkeese]

1) isLeftToRight() is known public method, looks simply and sounds natively. effectiveDir is something internal and (in my opinion) user should not be aware of it.

It's true that isLeftToRight() is a known public method, but since we haven't released v1.0, we are allowed to change the API, by removing that method and replacing it with a property.

I don't know why you think that isLeftToRight() is a better public API than effectiveDir, but the reason I want the public API to be a property rather than a method is because widgets can react to property changes in refreshRendering() and computeProperties().

2) if some widget has explicitly defined dir and somebody changes its value to empty, we need recompute value of effectiveDir once again

I'm not concerned with the performance in that case because it's very unlikely.

[semion1956]

It's true that isLeftToRight() is a known public method, but since we haven't released v1.0, we are allowed to change the API

I want only to note, that isLeftToRight() is known in v2.0 too. It is used in delite and deliteful in many places. This is one more reason why I wanted to keep it.

I don't know why you think that isLeftToRight() is a better public API than effectiveDir

Announcing effectiveDir as public API we give permission to direct setting of this property - programmatically and declaratively. So we can have problems with wrong or empty value of effectiveDir or collision with definitions of both dir and effectiveDir with non-emty values.

Anyway, I tried to perform your requirements in the latest commit. I keep temporary isLeftToRight() and will remove it after changes in deliteful.

[wkeese]

OK thanks, that looks better to me.

About getInheritedDir():

• Can it just return the inherited value rather than setting effectiveDir? That should simplify the code in computeProperties() to this.effectiveDir = this._get("dir") || this.getInheritedDir();
• Our rule is to prefer dcl.superCall() over dcl.after(), but ISTM getInheritedDir() shouldn't need either, as Bidi#getInheritedDir() can just override Widget#getInheritedDir().
• AFAIK you shouldn't need the if (!!window) check

About effectiveDir:

• We should document that it's read only, and the possible values.
• If we guarantee that the value is either "ltr" or "rtl" in lowercase, and never an empty string, then it might slightly simplify the other code?

[wkeese]

PS: Oh also, as I said in https://github.com/ibm-js/delite/pull/363#issuecomment-71045175 this should be under a different has() flag: has("inherited-dir").

[semion1956]

I thought, that in ibm-js/deliteful#462 we agreed, that all bidi advances features will be under one "bidi" flag. Anyway I updated code as you require.

[wkeese]

Thanks for the updates. Can you also update docs/g11n.md?

[semion1956]

Can't you reduce these 5 lines (113 - 117)...

Id dir is set with wrong (but not empty) value, getInheritedDir() should be called.

[semion1956]

Done.

[wkeese]

OK, thanks! I pushed your changes as fbcff47f42c53911a46248b45f45c1a868df4ec8 after making the following fixes/changes:

• fixed the remaining jshint errors
• fixed error in Widget#refreshRendering(): this.style.direction = this._get("dir") should run whenever "dir" has changed, not when "effectiveDir" has changed.
• removed isLeftToRight(); when deliteful is upgraded to run against delite 0.7.0 (or delite master) then it will need to switch to using effectiveDir
• added JSDOC for getInheritedDir() and made sure it always returns lowercase

I think there might be a remaining bug where the notifyCurrentValue("dir") should [also] happen on attachedCallback() rather than postRender(), for the unusual case where a widget is created and not immediately attached to the dom.