postiffm / bibledit-desktop

Desktop version of Bibledit
GNU General Public License v3.0
4 stars 6 forks source link

Floating window: Optimize the formatting of the title. #70

Closed rluzynski closed 5 years ago

rluzynski commented 5 years ago

Please do not yet merge this pull request because most likely it will break the things in gtk3 branch. I should prepare another pull request for gtk3.

The text of label_title is set only once in title_change() while title_setfocused() only updates the color and font weight of the label. All label_title properties which do not change when a selected floating window changes are moved to the XML template.

postiffm commented 5 years ago

Hi Rafal, this is excellent. It looks much better than before.

I found one problem, which is almost certainly in my code. If you View | Analysis window, you will see that title_setfocused does not seem to work properly on it. I mean, it sets the focus, but then it never takes the blue and bold away when focus is shifted to another window. The other windows seem to work fine when they lose focus.

The bug is probably in windowtabbed.cpp, and maybe has something to do with line 182. Each tab that is created calls parent->connect_focus_signals, but now I am not sure if that is correct. Do you know what is happening?

rluzynski commented 5 years ago

[…] I found one problem, which is almost certainly in my code. If you View | Analysis window, you will see that title_setfocused does not seem to work properly on it. I mean, it sets the focus, but then it never takes the blue and bold away when focus is shifted to another window. The other windows seem to work fine when they lose focus.

I confirm this. But this is not related with my current contribution because it was the same before. I will take a look at that problem but I would like to give this pull request a higher priority.

postiffm commented 5 years ago

I agree with you. Besides, I think I found the error, in mainwindow.cpp. Not sure if that is the best place for this code, but it is where it always has been. Around line 6666. Fixed in 9e5c41d.

postiffm commented 5 years ago

Rafal, this seems to work well on both the master and gtk3 branches.

rluzynski commented 5 years ago

Thank you for testing this before I did. Indeed, it works well with gtk3. I asked not to merge because I was not sure. Since it works then feel free to merge it with master and then rebase gtk3 branch against the current master.

This pull request uses some features which are deprecated in GTK 3.x (deprecated does not mean not working) therefore I will open another pull request for gtk3. But for now, as it does not break gtk3, it is fine to merge.

postiffm commented 5 years ago

Also merged and rebased gtk3 branch. See a8fbb2c.