- User Since
- Oct 11 2017, 3:04 AM (97 w, 11 h)
Cool, everything looks fine on the docfx side. Thanks for the screenshots, I'm away from my work computer :)
I didn't know tuples existed in C# so I cannot comment much.
However, anything to make usage of EFL under C# more natural sounds like a good idea to me. Multivalued eo properties did not generate C# properties before, so I see no harm in this patch.
Mon, Aug 19
Sun, Aug 18
As @raster points out, it is the job of eolian_gen to take care of any language-specific quirks. For example, for C# we could add a C prefix to all classes just like we add I to interfaces (and Evt to events).
We initially played with upper-case for classes and lower-case for properties, and things like this, but it wasn't natural for C# (and impossible for Python on Windows).
At some point we started renaming classes and properties instead of letting each language eolian_gen handle language-specific problems (and I think T6847 was the initial culprit).
Sat, Aug 17
Ah, ok, copies of each other. I thought they had copies in Layout. Thx!
Docs are a bit convoluted, but I won't block this patch because of it. Most efl docs need a rewrite.
Commit message says nothing about efl_ui_list 😡
Just for my personal understanding, what is going on here?
I see that 3 item parts are being mapped onto 2 layout parts. Isn't there an overlap?
Fri, Aug 16
Enums need a doc for themselves, besides the doc for each individual value. Same thing for the structs. Why isn't this triggering build breaks???
Also, please always add a period at the end of sentences. Sometimes doc generators merge multiple sentences and they make no sense of there is no punctuation. I think I have not seen any period in efl_ui_position_manager_entity.eo...
Ok ok :)
Thu, Aug 15
My last comment! EnableDirectTextInput?
hmmmm... I don't like any of them, I am a horrible person :)
- EditableText looks like this property should be text, not a boolean
- TextEditable sounds weird in English
- TextIsEditable sounds best to me, but we have nothing like this in the current API
- Editable sounds like the opposite of Enabled
Wed, Aug 14
I'll leave the technical details to the c# people, but I think it is a bit late in the day to change the c# bindings syntax now. Not impossible, though.
All name clashes were already solved a long time ago by renaming properties named like classes. Only these three remained (Key, Hold and Text) because they were in a blacklist in the c# generator and I forgot about them.
Hmmmm... not that exotic, in those languages, a method named like the class is reserved for the constructor of the class, isn't it?
They clash because c# will create a property named Text inside a class named Text, which is forbidden.
I'm OK with wraparound but I really think editable_text is better than text_editable (adjective before noun).
Another option is text_is_editable.
Tue, Aug 13
Any chance we can explain why this is so? Do we have at least an excuse?
Current c# generator ignore setter and getter docs when there's also a property doc. I would move or remove the setter and getter docs.
Mon, Aug 12
What are the (user) hints?
Maybe using links like @.hint_size_min helps understanding this class a bit.
There's a tutorial planned for this.
someone please update this cryptic comment
/* Add font name last to save evas from multiple loads */
how this is going to help saving multiple load.
Fri, Aug 9
What?! Those two types exist??? /o\
Wed, Aug 7
It might be worth printing a warning, in case the multiple inheritance was unintentional...
Where do you mean? The .eo.h files? I think the DocFX pages do have that info. The C reference pages were generated 2 years ago and are a bit forgotten...
Sorry, I don't understand what you mean :/
Tue, Aug 6
This selectes whether the scrollbar appears at the bottom or at the side of the scroller, but also whether the scrollbar is horizontal or vertical, no? I think it is very related to orientation.
Now that D9299 has landed the idea from T7924#137454 (above) can be implemented, I think.
Mon, Aug 5
OK, I should not be doing reviews while on holidays :)
r * 76 causes overflow.
Test case is very poor, and it should be a unit case, testing far more cases. Ideally a whole ramp.
EINA_UNUSED vars are actually used.
Sat, Aug 3
Fri, Aug 2
- scalable: hmmm.. there's a weird relationship there, since the docs say if $false, the internal image is not scaled up no matter what the scale type is.. So these two booleans seem to go further than the scale_type in Efl.Gfx.Image. Not sure if the implementation supports it, though.
- align: yeah, Efl.Gfx.Arrangement controls the alignment of the content inside the container so it fits nicely here. It also has a padding property that wouldn't be used.
- drop: it should go away, sure. We already have @beta events, but I am not sure how the mono docs react to them.
Sure, yeah, feel free to commandeer :)
key_sym, for consistency.
Strictly speaking.... yeah, I guess so /o\
But there's bind_to_theme and bind_to_object methods. What would you do with the object ones?
This patch will need rebasing if we decide a new name for input_key in D9483.
Feel free to commandeer this patch, rebase and land it, since I might not be available in the following weeks.
I see no problem here (besides the class being already stable).
We did not discuss which name to use to replace key. For consistency, if we use the input_ prefix for this property, we should use it for all other properties in this class. This solution would not change the C API and therefore would not break compatibility, as @YOhoho pointed out.
How would we do that? We make this release without user,changed events and add them afterwards? I'm OK with this but are these events required by any app right now?
Explaining things is my job! :D
Makes sense. Is there any information in the commit that introduced de 0.1s for the first time about why they choose such a low value?
Thu, Aug 1
Works as expected.
Works well after the next two patches :P
Curious. Why was that there in the first place?
Now I realize that the spin_button elm_test has never worked as intended :/
Actually, this break the Efl.Ui.Spin elm_test, because it is calling efl_ui_range_step_get and _set on the spin in multiple places.
Maketh sense, buildeth and paseth tests.
Aaahhhhhhh COOOOL, you used my new method! :D
DOUBLE_CLICK_TIME is the time required for two consecutive clicks to be considered together.
After the second click, you have 0.1s to click again and get a triple click.
After the third click, you have 0.1s again to click a forth time, etc.
Is there a use case that developers use the value "repeated"?
There are some apps which use triple-clicking (like selecting a word with double-click, and a whole paragraph with triple-click).
I don't know yet how styles work, so here's my less than two-cents worth:
- It seems obvious that the user should be able to change a widget's style. No? Or is that meant to be changed through other APIs, like set_layout_direction()?
- style_set is meant to be called inside the constructor. How's the spinner cxx example supposed to work?
- All C# widgets can change style at construction time through a constructor parameter. Does something similar exist for cxx?
- As I recall, styles are free-form strings, defined by themes, widgets, extensions and whatnot. How are we gonna document that? Do we need a runtime tool that lists available styles?
vpaths looks like a very convenient thing to have. We should also be consistent, so if we allow them in some APIs, we should allow them everywhere. This seems to be already the case, as @bu5hm4n said, right?
Wed, Jul 31
Reason for this is that Spin is not interactive, even if it implements the interface for it. Maybe something we should resolve in future.
The EO docs could use some love...
What is the issue?
hmmm... in the radio case, which widget would perform this centralization? and which ones are emitting the selected and unselected events?
Names containing the word View should be reserved for MVVM widgets. Otherwise it's going to be very confusing.
Other than that, I have no strong opinion. There cannot be name conflicts between Legacy and Unified APIs since they are in different namespaces.
We have all worked way too much on this widget now and we are all sick of it.
I think steady only makes sense for user-generated movements.
Yeah, I think so too.
Thanks for a very detailed report!
Fix meaning of min and max positions.
Everything fine now!
Tue, Jul 30
OK, I'm sold (you could have written that in the commit message to begin with). I won't be able to do a proper technical review, though.