Page MenuHomePhabricator

elementary: improve focus memory for Efl.Ui.CollectionView.
ClosedPublic

Authored by cedric on Nov 14 2019, 5:20 PM.

Details

Summary

This patch will make the CollectionView remember at all time the last
focus object and the last item in the list.

Diff Detail

Repository
rEFL core/efl
Branch
T8351-devs/cedric/cv-focus
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 14926
cedric created this revision.Nov 14 2019, 5:20 PM
cedric requested review of this revision.Nov 14 2019, 5:20 PM

This looks overall good. However, it took me quite a bit to get the difference of last and previous.

src/lib/elementary/efl_ui_collection_view.c
84

Can you add here a comment that last refers to the order defined by the objects index. And previously refers to the *previously* focused element (speaking of a time-axis, not the index based ordering).

503

Can this operation be very heavy ? I am wondering if it makes more sense to store the index next to the object. But that can still be evalualted later on.

bu5hm4n requested changes to this revision.Nov 17 2019, 2:56 AM
This revision now requires changes to proceed.Nov 17 2019, 2:56 AM
cedric planned changes to this revision.Nov 20 2019, 9:45 AM
cedric added inline comments.
src/lib/elementary/efl_ui_collection_view.c
84

Sure.

503

It could, but it shouldn't and maintaining a local index is a pain as you now have to know the model behavior and mimic it (Sorted insert, append, prepend and so on). I think it is best to let as much of this to the model, or we are likely going to shoot ourself in the foot.

cedric updated this revision to Diff 27011.Nov 20 2019, 10:07 AM

rebase and add comments.

bu5hm4n accepted this revision.Nov 26 2019, 6:22 AM
bu5hm4n added inline comments.
src/lib/elementary/efl_ui_collection_view.c
503

Okay. Lets see if we need more action here later on ... :)

This revision is now accepted and ready to land.Nov 26 2019, 6:22 AM
bu5hm4n requested changes to this revision.Nov 26 2019, 6:30 AM

Mhm, this still has a bug. Open "Efl.Ui.Collection_View" focus element 49 and press down. You will see an error, this should not be.

src/lib/elementary/efl_ui_collection_view.c
763

This is A.

832

This is equal to "A". Can you refactor this into a function?

This revision now requires changes to proceed.Nov 26 2019, 6:30 AM
bu5hm4n requested changes to this revision.Dec 18 2019, 12:09 AM

Looks like this was not updated, so this is still bugging.

This revision now requires changes to proceed.Dec 18 2019, 12:09 AM
cedric updated this revision to Diff 27640.Dec 18 2019, 4:56 PM

rebase and refactor.

bu5hm4n requested changes to this revision.Dec 19 2019, 1:22 AM

I still get the same error when focusing "Initial index: 49" and pressing down :(

This revision now requires changes to proceed.Dec 19 2019, 1:22 AM

I still get the same error when focusing "Initial index: 49" and pressing down :(

This is another problem, see D10928 for fixing that one.

bu5hm4n accepted this revision.Dec 27 2019, 2:35 AM
This revision is now accepted and ready to land.Dec 27 2019, 2:35 AM
Closed by commit rEFL6756485476c0: elementary: improve focus memory for Efl.Ui.CollectionView. (authored by cedric, committed by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyDec 27 2019, 2:37 AM
This revision was automatically updated to reflect the committed changes.