search

flutter-tizen / engine

The Flutter engine
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
6 stars 19 forks source link

Introduce TizenViewElementary #292

Closed JSUYA closed 2 years ago

JSUYA commented 2 years ago

There are two ways to draw FlutterTizenView in Tizen. (TizenWindowElementary and TizenWindowEcore) Both ways are drawing on Window component(surface).

We try to draw FlutterTizenView on the appropriate component provided by Tizen UIFW and support it to be place with other components. To do that, we created a TizenView class on the same level as TizenWindow and abstracted common methods to TizenViewBase. we can add FlutterTizenView to other UIFW of Tizen while adding TizenView{?}(maybe Tizen NUI).

 TizenViewBase -> TizenView -> TizenViewElementary
                              -> TizenView{?}
                 -> TizenWindow -> TizenWindowElementary
                                -> TizenWindowEcore

related PR : https://github.com/flutter-tizen/flutter-tizen/pull/373

bbrto21 commented 2 years ago

In my opinion, I would recommend that you rebase this PR to the master after #291 is merged.

swift-kim commented 2 years ago

The name TizenBaseHandle is somewhat ambiguous. We'll need to discuss about the class design and hierarchy.

JSUYA commented 2 years ago

The name TizenBaseHandle is somewhat ambiguous. We'll need to discuss about the class design and hierarchy.

Do you have a recommended class name and hierarchy? I've been discussing names and hierarchy with @bbrto21. Please let me know if there is a better way

JSUYA commented 2 years ago

contribute to https://github.com/flutter-tizen/flutter-tizen/issues/351

JSUYA commented 2 years ago

In my opinion, I would recommend that you rebase this PR to the master after https://github.com/flutter-tizen/engine/pull/291 is merged.

@bbrto21 I rebased your #291

The name TizenBaseHandle is somewhat ambiguous. We'll need to discuss about the class design and hierarchy.

@swift-kim I renamed it to TizenViewBase as we discussed. And I cleaned up the level of abstraction. More reviews are needed. Could you please review it?

swift-kim commented 2 years ago

Please don't mark my comment as "resolved" by yourself. I'll do that when necessary.

If I checked your code and didn't close the corresponding comment, there's always a reason for it.

JSUYA commented 2 years ago

Please don't mark my comment as "resolved" by yourself. I'll do that when necessary.

If I checked your code and didn't close the corresponding comment, there's always a reason for it.

um... I think of Resolved as a simple expression of the comment("Done") that your review has been accepted. If you have a reason just do Unresolved again. If it is the rule of flutter-tizen that the reviewer checks Resolved, I will follow it.

swift-kim commented 2 years ago

I told because you were keeping closing comments that I had set as "unresolved". Please leave a simple comment such as "Done" if the issue has been resolved.

bbrto21 commented 2 years ago

@swift-kim Do you have anything more to discuss?

bbrto21 commented 2 years ago

I told because you were keeping closing comments that I had set as "unresolved". Please leave a simple comment such as "Done" if the issue has been resolved.

Is this a common practice? for those who have just joined the project this can be an inconvenience. If this is common practice, or if everyone agrees on it, it looks like it needs an addition to the development guide.

swift-kim commented 2 years ago

@bbrto21 No, my comment was specifically for him. It's not a rule that applies to everyone.

Do you have anything more to discuss?

It seems there are still some unresolved issues that have not been answered by the author.