Page MenuHomePhabricator

theme: hide next and prev buttons when title is hidden
ClosedPublic

Authored by bu5hm4n on Dec 5 2018, 3:15 AM.

Details

Summary

This ensures that a button is hidden when not beeing able to be visible.

fixes T6891

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bu5hm4n created this revision.Dec 5 2018, 3:15 AM
bu5hm4n requested review of this revision.Dec 5 2018, 3:15 AM

@bu5hm4n

How about changing the state of "elm.swallow.prev_btn" and "elm.swallow.next_btn" in naviframe.edc instead of code change?

e.g. if title is not enabled, then title is hidden so "elm.swallow.prev_btn" and "elm.swallow.next_btn" are hidden as well.

@Jaehyun_Cho I can do that, sure, however, this would mean that any other theme would also need this fix ... :) Is that acceptable ?

@bu5hm4n

The reason why I want to modify theme is because there is no rule prev_btn and next_btn should be inside the title area.

This is totally related to framework's and application's UX.

So if some application's UX puts prev_btn and next_btn regardless of title area, then the prev_btn and next_btn should not be hidden when title is not enabled.

bu5hm4n planned changes to this revision.Dec 18 2018, 4:37 AM

Okay that makes sense! :)

bu5hm4n updated this revision to Diff 17968.Dec 18 2018, 6:08 AM
bu5hm4n retitled this revision from elm_naviframe: disable not used buttons to theme: hide next and prev buttons when title is hidden.
bu5hm4n edited the summary of this revision. (Show Details)

make the change in the theme

This comment was removed by bu5hm4n.
Jaehyun_Cho requested changes to this revision.Dec 18 2018, 8:57 PM

I think that the added program signal is duplicated.

How about changing state name of "elm.swallow.prev_btn" and "elm.swallow.next_btn" to "title-hidden"?
And how about changing the state whenever title visibility is changed?

data/elementary/themes/edc/elm/naviframe.edc
378

how about changing this state name to "title-hidden"?

399

how about changing this state name to "title-hidden"?

465–467

how about adding the followings?

target: "elm.swallow.prev_btn";
target: "elm.swallow.next_btn";

480

how about adding the followings?

target: "elm.swallow.prev_btn";
target: "elm.swallow.next_btn";

488

how about adding the followings?

target: "elm.swallow.prev_btn";
target: "elm.swallow.next_btn";

498–500

how about adding the followings?

target: "elm.swallow.prev_btn";
target: "elm.swallow.next_btn";

504

This signal program already exists above.

So I think it would be better remove this program.

522

how about changing this state name to "title-hidden"?

533

how about changing this state name to "title-hidden"?

This revision now requires changes to proceed.Dec 18 2018, 8:57 PM

Well, I tried to keep the patch as simple and small as possible, and not change unrelated things like the signal names, if changeing them is fine for you, then i can also go this path :)

What do you prefer more ?

data/elementary/themes/edc/elm/naviframe.edc
480

I can surly do that - however, whenever the buttons are setted, there is a call to elm,state,title,show , so i skipped this :)

504

This was only added for the purpose of a different state name :)

bu5hm4n updated this revision to Diff 17976.Dec 19 2018, 5:25 AM

update to latest review

Thank you for update :)

Could you please add state change for prev_btn and next_btn when title becomes visible with transition?

data/elementary/themes/edc/elm/naviframe.edc
480

The following code should be added here as well.

target: "elm.swallow.prev_btn";
target: "elm.swallow.next_btn";

Because the visibility of title area can be changed by calling elm_naviframe_item_title_enabled_set().

If there is a transition to show title, then "elm,action,title,show" is emitted.
If there is no transition to show title, then "elm,state,title,show" is emitted.

As a result, if title becomes visible with transition after being hidden, then the prev_btn and next_btn should change its state again. :)

bu5hm4n updated this revision to Diff 18008.Dec 19 2018, 11:01 PM

update last bit

Thx for the explanation! :)

Jaehyun_Cho accepted this revision.Dec 20 2018, 4:29 AM

Thank you :)

This revision is now accepted and ready to land.Dec 20 2018, 4:29 AM
Closed by commit rEFLe15d696372a8: theme: hide next and prev buttons when title is hidden (authored by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyDec 20 2018, 5:05 AM
This revision was automatically updated to reflect the committed changes.