fix bug with tabview button appearances #10657
Conversation
p9malino26
requested review from
oetiker,
johnspackman,
level420,
cboulanger,
derrell,
hkollmann,
lucapivato and
goldim
as code owners
There was a problem hiding this comment.
I disagree that it is a bug. The first tab is first tab. Why it should move own state to other button otherwise if the first or last button is removed but in case of removal everything is fine. Also could you create an issue next time for discussion please and code example attached to it also would be great bc it is time for other devs to reproduce the issue.
Because it looks weird from the users point of view - if the first tab's visibility is excluded, then it means that the user's "first" tab does not look like the first when the theme's appearance needs to style that "first" tab differently (eg due to borders etc). When a tab widget has a state like "first", this is only something which is passed to the appearance, it seems reasonable that this state is saying which tab "appears first" |
|
@johnspackman For me first tab is tab which first in array of tabs and not in visible ones. Maybe users' apps are based on old behavior and I may consider this like broken BC, am I right? I would like hear everyone's opinion on this problem. Maybe to create separate discussion? |
|
@p9malino26 In old implementation if I have 3 buttons, removed first and third, central button will have both states. In yours only lastTab. |
|
by all means create a separate discussion, or we can carry on here; I guess my point is that the state of "first" is only for the appearance, and if the tab is excluded then the appearance will never see a "first" tab (without this PR), in which case what would be the point of a "first" state. |
| page.removeState("lastTab"); | ||
| } | ||
| firstPage?.addState("firstTab"); | ||
| firstPage?.removeState("lastTab"); |
There was a problem hiding this comment.
You already remove this state in cycle. Btw if you get rid of these two lines you will fix the problem which I told: one tab will have both states.
| firstPage?.removeState("lastTab"); | ||
|
|
||
| lastPage?.addState("lastTab"); | ||
| lastPage?.removeState("firstTab"); |
|
@johnspackman I can see that it is a bug actually. Classic Theme: |
|
@p9malino26 just fix what I suggested above and I gonna approve it. |
* fix bug with tabview button appearances * remove redundant code --------- Co-authored-by: Patryk Malinowski <pmalinowski@vmn.digital> Co-authored-by: Henner Kollmann <henner.kollmann@gmx.de> Thanks @p9malino26 !
This PR fixes an issue with appearances in a qx.ui.tabview.TabView, where if the first tab has its
tabVisibilityset toexcluded, the first visible tab will not have the statefirstTab, which will break appearance.