search

oracc / nammu

Oracc GUI
GNU General Public License v3.0
12 stars 10 forks source link

Check current status of split view before toggling #384

Closed giordano closed 5 years ago

giordano commented 5 years ago

Fix #353

giordano commented 5 years ago

I added the improvements suggested by Anastasis (slightly changed, but the bulk was good, thanks a lot!), I only need to add some tests :see_no_evil:

giordano commented 5 years ago

I think this is good for review now. I added a few tests, that are eventually passing on Travis (after several restarts :roll_eyes:)

raquelalegre commented 5 years ago

Also we can add a separator in the Window menu between the hide/show options and the toggling options.

raquelalegre commented 5 years ago

Found a small logic inconsistency:

  1. Open Nammu
  2. Choose Toggle Arabic pane
  3. Open arabic.atf
  4. The arabic pane and the arabic part of the file are not displayed

If you try to open the arabic.atf file again, it will work fine. It just doesn't seem to work when opening from arabic toggle mode.

Same happens when opening from another toggle mode, so this problem is not only particular to arabic toggle.

giordano commented 5 years ago

I think I addressed all comments.

Regarding the last one, my solution has been to add a force argument to NammuController.arabic and call arabic(force=True) when opening an Arabic file, see https://github.com/oracc/nammu/pull/384/commits/43aa2caec405c81a38aaa3da783358236ee42d21#diff-2bd150ec3e0b82e145346223be946f73. I couldn't find any reliable control for fixing this.

I find the whole toggling mechanism a bit perverse as it involves non-mutual-exclusive features to be toggled. Now there are functions to just do the thing (like atfAreaView.setup_edit_area_split and atfAreaView.setup_edit_area_no_split), and there are some checks before actually toggling, instead of blindly toggling whatever it was before.