D22988: Fix incorrect Kickoff tab bar layout for vertical panels
lisin added a comment. Yes, getting into it was pretty straightforward and quick, KDE team did a great job! REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D22988 To: lisin, #plasma, hein, ngraham Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22988: Fix incorrect Kickoff tab bar layout for vertical panels
ngraham added a comment. Very nice job! May it be the first of many. :) I hope the process wasn't too difficult. We're trying to make it smoother for new contributors. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D22988 To: lisin, #plasma, hein, ngraham Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22988: Fix incorrect Kickoff tab bar layout for vertical panels
lisin added a comment. Thank you, this was my first ever contribution to FOSS! I have submitted the fix for the TabBar here: https://phabricator.kde.org/D23036 I'm not quite sure about who to add as reviewers though. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D22988 To: lisin, #plasma, hein, ngraham Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22988: Fix incorrect Kickoff tab bar layout for vertical panels
ngraham added a comment. Now we just need that plasma-frameworks patch to also fix https://bugs.kde.org/show_bug.cgi?id=395390. :) Let me know if you need a hand. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D22988 To: lisin, #plasma, hein, ngraham Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22988: Fix incorrect Kickoff tab bar layout for vertical panels
This revision was automatically updated to reflect the committed changes. Closed by commit R119:2054d9be40bc: Fix incorrect Kickoff tab bar layout for vertical panels (authored by lisin, committed by ngraham). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22988?vs=63369&id=63371 REVISION DETAIL https://phabricator.kde.org/D22988 AFFECTED FILES applets/kickoff/package/contents/ui/FullRepresentation.qml To: lisin, #plasma, hein, ngraham Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22988: Fix incorrect Kickoff tab bar layout for vertical panels
ngraham added a comment. Thanks very much! REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D22988 To: lisin, #plasma, hein, ngraham Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22988: Fix incorrect Kickoff tab bar layout for vertical panels
lisin updated this revision to Diff 63369. lisin edited the summary of this revision. lisin edited the test plan for this revision. lisin added a comment. Removed line 435. REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22988?vs=63264&id=63369 REVISION DETAIL https://phabricator.kde.org/D22988 AFFECTED FILES applets/kickoff/package/contents/ui/FullRepresentation.qml To: lisin, #plasma, hein, ngraham Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22988: Fix incorrect Kickoff tab bar layout for vertical panels
hein added a comment. ... I'm not the author or maintainer of this code, but I had a look anyway :). @lisin, I agree with you that the sizing bug should be fixed in TabBar instead, in plasma-frameworks.git. It's very unorthodox to call a property change handler as a function, and it's not going to fix this for other potential users of the component. Otherwise the patch looks good. Could you resubmit it without line 435, and then submit a seperate patch to plasma-frameworks? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D22988 To: lisin, #plasma, hein, ngraham Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22988: Fix incorrect Kickoff tab bar layout for vertical panels
ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Thanks very much for the patch! This fixes both issues for me and looks conceptually like an appropriate fix to me, but I'm not the original author or maintainer of this code as @hein is, so let's wait for his review. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D22988 To: lisin, #plasma, hein, ngraham Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22988: Fix incorrect Kickoff tab bar layout for vertical panels
lisin added a comment. I also couldn't reproduce the gray overlay (which is caused by `tabBarSeparator` having a wrong size and taking the whole view - yesterday I could reproduce it but no more) that is shown in the comment 16 here: https://bugs.kde.org/show_bug.cgi?id=395390#c16 So it may still be present. Currently, I can't see any separator but it's a different issue. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D22988 To: lisin, #plasma, hein, ngraham Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22988: Fix incorrect Kickoff tab bar layout for vertical panels
lisin created this revision. lisin added reviewers: Plasma, hein, ngraham. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. lisin requested review of this revision. REVISION SUMMARY This fixes the incorrect initial positioning of the tab bar (first tab is placed out of bounds) when Kickoff is in a vertical panel that persists until the user selects another tab manually. BUG: 395390 And the broken layout of the tab bar (tab bar takes the whole view) when a panel is changed from horizontal to vertical that persists until plasmashell is restarted. BUG: 393888 TEST PLAN BUG: 395390 Place Kickoff in a vertical panel. Restart plasmashell and open Kickoff. Before fix: first tab is positioned out of bounds (y<0). After fix: first tab is positioned correctly (y=0). BUG: 393888 Change panel orientation from horizontal to vertical. Open Kickoff. Before fix: tab bar fills the whole view making the Kickoff unusable even if you make the panel horizontal again. After fix: tab bar has the correct size. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D22988 AFFECTED FILES applets/kickoff/package/contents/ui/FullRepresentation.qml To: lisin, #plasma, hein, ngraham Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D22988: Fix incorrect Kickoff tab bar layout for vertical panels
lisin added a comment. Line `onHeightChanged: onWidthChanged()` fixes BUG: 395390 I'm not sure if this is the best solution. For some reason, `plasmacomponents/qml/TabBar.qml` lacks an `onHeightChanged()` function but it has `onWidthChanged()` that seems to do what needs to happen here. Maybe `TabBar.qml` should be changed instead. The rest of the changes is for BUG: 393888 It seems to me that the removed code was a more clean way to do this, but it didn't update the values without restarting. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D22988 To: lisin, #plasma, hein, ngraham Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
