Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show/hide statusbar option #4029

Closed
wants to merge 1 commit into from
Closed

Conversation

takiz
Copy link

@takiz takiz commented Oct 30, 2015

Image of takiz
It's usefull for small displays.

@@ -1107,6 +1108,12 @@ void MainWindow::loadPreferences(bool configure_session)
toolBar->setVisible(false);
}

if (pref->isStatusbarDisplayed()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

statusBar()->setVisible(pref->isStatusbarDisplayed());

@glassez
Copy link
Member

glassez commented Oct 30, 2015

@takiz Your changes are incomplete!
Status bar displays some information, which regularly updates. It makes no sense to do this when the statusbar is hidden.

<bool>true</bool>
</property>
<property name="text">
<string>Status bar</string>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bar -> Bar

And your screenshot doesn't show the correct capitalized menu items.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

& missing?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Status bar" -> "Status Bar"

& missing?

You can give one if you want.

And screenshot the latest git version, not an outdated old version.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image of takiz

@takiz
Copy link
Author

takiz commented Oct 31, 2015

Status bar displays some information, which regularly updates. It makes no sense to do this when the statusbar is hidden.

In that case resource economy is low. It makes no sense.

@glassez
Copy link
Member

glassez commented Nov 1, 2015

@takiz You shouldn't create separate commits to fix the other commits from the current PR. This will pollute the qBittorrent history. Maintainer will not merge this.

@sledgehammer999
Copy link
Member

@takiz I presume you don't know much about git so I am giving you a few hints here on how to squash commits into one.

  1. If you're on Windows you might want to setup a graphical editor for git. Terminal may not work correctly in interactive mode. (search google on how to do that)
  2. Issue git rebase -i <hash>. This is the hash of the first(oldest) commit you want to manipulate. All the commits that follow it(newer commits) will be present in the list-> After you issue the command a file will open in your graphical editor. That file contains very good information on what is going on. Choose the "fixup" action for the relevant commits. (you'll understand once you see the file). For more info search google for "git interactive rebase".
  3. After you're done merging commits, you need to update your branch that this PR is depended on. But because you re-wrote git history you need to do a forced push. Something like this git push --force origin statusbar. After that this PR will autoupdate.

@sledgehammer999
Copy link
Member

Also update your code to follow our style guide: https://github.com/qbittorrent/qBittorrent/blob/master/CODING_GUIDELINES.md
And at the first post of #2192 you can find a config for the uncrustify tool if you want to use that.

@takiz
Copy link
Author

takiz commented Nov 6, 2015

@sledgehammer999 done.

@@ -1352,6 +1355,13 @@ void MainWindow::on_actionTop_tool_bar_triggered()
Preferences::instance()->setToolbarDisplayed(is_visible);
}

void MainWindow::on_actionShow_Status_bar_triggered()
{
bool is_visible = static_cast<QAction*>(sender())->isChecked();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use camelCased names.

@ngosang
Copy link
Member

ngosang commented Nov 7, 2015

tested and looks good in linux with qt5.

void MainWindow::on_actionShowStatusbar_triggered()
{
bool isVisible = static_cast<QAction*>(sender())->isChecked();
QMainWindow::statusBar()->setVisible(isVisible);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the status_bar member variable

@sledgehammer999
Copy link
Member

I think you should reimplement the showEvent() and hideEvent() in our StatusBar subclass and stop the timer when it is hidden. When shown you'll need to do a call to StatusBar::refreshStatusBar() too and start the timer (don't forget to remove those 2 commands from the constructor).

@glassez
Copy link
Member

glassez commented Nov 24, 2015

I think you should reimplement the showEvent() and hideEvent() in our StatusBar subclass and stop the timer when it is hidden. When shown you'll need to do a call to StatusBar::refreshStatusBar() too and start the timer (don't forget to remove those 2 commands from the constructor).

@sledgehammer999, what timer do you mean?

@sledgehammer999
Copy link
Member

The StatusBar class has a timer inside it and is used to update the speed info.

@ngosang
Copy link
Member

ngosang commented Dec 22, 2015

@takiz any progress on this? could you rebase? Thanks.

@ngosang
Copy link
Member

ngosang commented Apr 16, 2016

@takiz ping.

@GUImodder
Copy link

Hi,

i just wanted to ask when/if this option is going to be introduced in the stable releases? I just installed v. 3.3.4 and there is no option to hide the statusbar yet!
Best regards

@GUImodder
Copy link

Who decides whether this feature will be integrated or not? I mean: Whats the harm? Giving people more option can hardly be a bad thing?

@GUImodder
Copy link

Ok Thanks...and WHEN will he decide on this?

@GUImodder
Copy link

Ok thats sad...alot of users will jump ship in that case..

@thalieht
Copy link
Contributor

thalieht commented Aug 4, 2016

There is always the option for anyone to take the PR, fix it and make a new PR.

@ngosang
Copy link
Member

ngosang commented Aug 23, 2016

@takiz ??

@glassez
Copy link
Member

glassez commented May 25, 2017

I will finish it, if you do not mind.

@glassez glassez self-assigned this May 25, 2017
@glassez
Copy link
Member

glassez commented May 25, 2017

Closing now. I'll open new PR with updated code.

@glassez glassez closed this May 25, 2017
glassez added a commit that referenced this pull request May 27, 2017
Add show/hide statusbar option (Supersede #4029)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants