Skip to content

Commit 2773e45

Browse files
trflynn89awesomekling
authored andcommitted
UI/Qt: Consistently place the tab close button across platforms
Before the recent Qt overhaul, we had no control over the position of the tab close button (without drawing/maintaining it ourselves, which we now do). Qt automatically placed the tab close button on the left side of the tab bar on macOS, and as such, we had to go out of our way to place the audio state button on the right side. See: dd54780 We now consistently place the audio state button on the left and the tab close button on the right. Qt still attempts to draw its own tab close button on the left, so we also explicitly set that to null initially.
1 parent 4a986de commit 2773e45

2 files changed

Lines changed: 18 additions & 34 deletions

File tree

UI/Qt/BrowserWindow.cpp

Lines changed: 17 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@
5050

5151
namespace Ladybird {
5252

53+
static constexpr auto AUDIO_STATE_BUTTON_POSITION = QTabBar::LeftSide;
54+
static constexpr auto TAB_CLOSE_BUTTON_POSITION = QTabBar::RightSide;
55+
5356
static QString reopen_recently_closed_action_text(Optional<WebView::RecentlyClosedEntry const&> entry)
5457
{
5558
if (entry.has_value() && entry->was_window)
@@ -564,8 +567,7 @@ void BrowserWindow::initialize_tab(Tab* tab)
564567
return new_tab.view().handle();
565568
};
566569

567-
m_tabs_container->set_tab_icon(m_tabs_container->index_of(tab), tab->tab_icon());
568-
create_close_button_for_tab(tab);
570+
initialize_tab_buttons(tab);
569571
}
570572

571573
void BrowserWindow::uninitialize_tab(Tab* tab)
@@ -741,38 +743,32 @@ void BrowserWindow::tab_favicon_changed(int index, QIcon const& icon)
741743
m_tabs_container->set_tab_icon(index, icon);
742744
}
743745

744-
void BrowserWindow::create_close_button_for_tab(Tab* tab)
746+
void BrowserWindow::initialize_tab_buttons(Tab* tab)
745747
{
746748
auto index = m_tabs_container->index_of(tab);
747749
m_tabs_container->set_tab_icon(index, tab->tab_icon());
748750

749-
auto* button = new TabBarButton(create_chrome_icon(ChromeIcon::Close, palette()));
750-
button->setToolTip("Close Tab");
751-
752-
auto position = audio_button_position_for_tab(index) == QTabBar::LeftSide ? QTabBar::RightSide : QTabBar::LeftSide;
751+
auto* close_button = new TabBarButton(create_chrome_icon(ChromeIcon::Close, palette()));
752+
close_button->setToolTip("Close Tab");
753753

754-
connect(button, &QPushButton::clicked, this, [this, tab]() {
754+
connect(close_button, &QPushButton::clicked, this, [this, tab]() {
755755
auto index = m_tabs_container->index_of(tab);
756756
request_to_close_tab(index);
757757
});
758758

759-
m_tabs_container->tab_bar()->setTabButton(index, position, button);
759+
m_tabs_container->tab_bar()->setTabButton(index, AUDIO_STATE_BUTTON_POSITION, nullptr);
760+
m_tabs_container->tab_bar()->setTabButton(index, TAB_CLOSE_BUTTON_POSITION, close_button);
760761
}
761762

762763
void BrowserWindow::update_tab_close_button_icons()
763764
{
764-
auto update_button = [this](int index, QTabBar::ButtonPosition position) {
765-
auto* button = m_tabs_container->tab_bar()->tabButton(index, position);
765+
for (int index = 0; index < m_tabs_container->count(); ++index) {
766+
auto* button = m_tabs_container->tab_bar()->tabButton(index, TAB_CLOSE_BUTTON_POSITION);
766767
if (!button || button->objectName() != "LadybirdTabButton")
767768
return;
768769

769770
if (auto* tab_bar_button = qobject_cast<TabBarButton*>(button))
770771
tab_bar_button->setIcon(create_chrome_icon(ChromeIcon::Close, palette()));
771-
};
772-
773-
for (int index = 0; index < m_tabs_container->count(); ++index) {
774-
update_button(index, QTabBar::LeftSide);
775-
update_button(index, QTabBar::RightSide);
776772
}
777773
}
778774

@@ -853,36 +849,35 @@ bool BrowserWindow::start_window_move()
853849
void BrowserWindow::tab_audio_play_state_changed(int index, Web::HTML::AudioPlayState play_state)
854850
{
855851
auto* tab = m_tabs_container->tab(index);
856-
auto position = audio_button_position_for_tab(index);
857852

858853
switch (play_state) {
859854
case Web::HTML::AudioPlayState::Paused:
860855
if (tab->view().page_mute_state() == Web::HTML::MuteState::Unmuted)
861-
m_tabs_container->tab_bar()->setTabButton(index, position, nullptr);
856+
m_tabs_container->tab_bar()->setTabButton(index, AUDIO_STATE_BUTTON_POSITION, nullptr);
862857
break;
863858

864859
case Web::HTML::AudioPlayState::Playing:
865860
auto* button = new TabBarButton(icon_for_page_mute_state(*tab));
866861
button->setToolTip(tool_tip_for_page_mute_state(*tab));
867862
button->setObjectName("LadybirdAudioState");
868863

869-
connect(button, &QPushButton::clicked, this, [this, tab, position]() {
864+
connect(button, &QPushButton::clicked, this, [this, tab]() {
870865
tab->view().toggle_page_mute_state();
871866
auto index = tab_index(tab);
872867

873868
switch (tab->view().audio_play_state()) {
874869
case Web::HTML::AudioPlayState::Paused:
875-
m_tabs_container->tab_bar()->setTabButton(index, position, nullptr);
870+
m_tabs_container->tab_bar()->setTabButton(index, AUDIO_STATE_BUTTON_POSITION, nullptr);
876871
break;
877872
case Web::HTML::AudioPlayState::Playing:
878-
auto* button = m_tabs_container->tab_bar()->tabButton(index, position);
873+
auto* button = m_tabs_container->tab_bar()->tabButton(index, AUDIO_STATE_BUTTON_POSITION);
879874
as<TabBarButton>(button)->setIcon(icon_for_page_mute_state(*tab));
880875
button->setToolTip(tool_tip_for_page_mute_state(*tab));
881876
break;
882877
}
883878
});
884879

885-
m_tabs_container->tab_bar()->setTabButton(index, position, button);
880+
m_tabs_container->tab_bar()->setTabButton(index, AUDIO_STATE_BUTTON_POSITION, button);
886881
break;
887882
}
888883
}
@@ -911,16 +906,6 @@ QString BrowserWindow::tool_tip_for_page_mute_state(Tab& tab) const
911906
VERIFY_NOT_REACHED();
912907
}
913908

914-
QTabBar::ButtonPosition BrowserWindow::audio_button_position_for_tab(int tab_index) const
915-
{
916-
if (auto* button = m_tabs_container->tab_bar()->tabButton(tab_index, QTabBar::LeftSide)) {
917-
if (button->objectName() != "LadybirdAudioState")
918-
return QTabBar::RightSide;
919-
}
920-
921-
return QTabBar::LeftSide;
922-
}
923-
924909
void BrowserWindow::open_next_tab()
925910
{
926911
if (m_tabs_container->count() <= 1)

UI/Qt/BrowserWindow.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ public slots:
164164
callback(*m_tabs_container->tab(i));
165165
}
166166

167-
void create_close_button_for_tab(Tab*);
167+
void initialize_tab_buttons(Tab*);
168168
void create_menu_bar_window_controls();
169169
void update_tab_close_button_icons();
170170
void update_menu_bar_style();
@@ -175,7 +175,6 @@ public slots:
175175

176176
QIcon icon_for_page_mute_state(Tab&) const;
177177
QString tool_tip_for_page_mute_state(Tab&) const;
178-
QTabBar::ButtonPosition audio_button_position_for_tab(int tab_index) const;
179178

180179
QScreen* m_current_screen { nullptr };
181180
double m_device_pixel_ratio { 0 };

0 commit comments

Comments
 (0)