search

postiffm / bibledit-desktop

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

Close buttons in the tabbed windows #92

Closed rluzynski closed 5 years ago

rluzynski commented 5 years ago

There are 2 commits in this pull request. The second one is an additional feature: close the floating window when its last tab is closed. This is what Firefox is doing, for example. I thought that this feature was reasonable. If not, feel free to reject it. Also, feel free to squash these commits if you find this necessary.

Please test this before you accept. I have tested and it seems OK but the memory management looks scary to me sometimes.

If you accept then please rebase the gtk3 branch against the new master. This code seems to work correctly in GTK 3.x and the results look nice but it may happen to use some deprecated features. It's not a big problem and I will contribute the updates to gtk3 when we finish this chunk of updates to master.

postiffm commented 5 years ago

The memory management is indeed a problem, and I knew it would be. There are some assumptions in the code where it does not expect the tabs to disappear. It works great for the concordance, I think, but not for the Analysis window. I'll take a deeper look at it.

postiffm commented 5 years ago

Sometime, we may want to have an option passed to the windowtabbed constructor to tell it that the tabs should or should not have a close button. For some notebooks, the close button may make sense on each tab, and in other notebooks, it may not make sense to have a close button.

The problem as it is currently implemented is that I have an assertion to make sure the tab exists, which it doesn't if the user closes it. I made a simple fix to this logic and we will see how it works.

postiffm commented 5 years ago

rluzynski, I cherry-picked your two commits and added a third to fix the small bug mentioned above. That closes this for the master branch. Now on to the gtk3 branch.

9d24972 (HEAD -> master, origin/master, origin/HEAD) Fix crash when an analysis window tab is closed 9e2df2a Close the floating window if its last tab has been closed. 62d1f5c Add close button to tabbed windows.

postiffm commented 5 years ago

The down-side of the git cherry-pick method of merging the pull request is that github does not automatically recognize that the changes were committed--I guess because they have new hashes.

rluzynski commented 5 years ago

Thank you. Maybe I'm wrong but I'm fine with the git history as it looks now, after git cherry-pick instead of git merge.