boxes: Disable stream change during edit.

[Abhirup-99]

Partially fixes #774

[prah23]

@Abhirup-99 Thanks for working on this 👍

I would suggest restructuring to use stream_box_edit_view solely to make the changes instead of the is_in_edit state, self.stream_write_box's nature can be overwritten inside stream_box_edit_view since the latter is extended from stream_box_view. You might have to mention self.header_write_box to include the overwritten stream box in the header box, but it would reduce the extra boolean you currently need, considering that we have a separate edit view for such specifications.

[Abhirup-99]

@Abhirup-99 Thanks for working on this

I would suggest restructuring to use stream_box_edit_view solely to make the changes instead of the is_in_edit state, self.stream_write_box's nature can be overwritten inside stream_box_edit_view since the latter is extended from stream_box_view. You might have to mention self.header_write_box to include the overwritten stream box in the header box, but it would reduce the extra boolean you currently need, considering that we have a separate edit view for such specifications.

Thanks for the review, I agree it's a valid concern that this should completely be done in the edit box but if I use self.header_write_boxinstead of the current approach, the corresponding syntax becomes
python self.header_write_box.widget_list[0] = .....
this would seem a bit more confusing than the current approach to anyone extending it, at least in my view because the entire first element gets overwritten. The current approach grants just seem better to me for the sake of understanding. I guess we can wait for Neil to suggest this part.

[prah23]

@Abhirup-99 I meant rewriting the stream_write_box to be a urwid.Text or so (as appropriate), and then use it in the same header_write_box as before. We wouldn't be changing the orientation or structure of the header_write_box. I believe the header_write_box's syntax or behavior shouldn't change.
Here's a snippet that might help make my suggestion clear for extending it in the stream_box_edit_view.

        self.stream_write_box = urwid.Text(u"")
        self.header_write_box = urwid.Columns(
            [self.stream_write_box, self.title_write_box],
            dividechars=1
        )

[Abhirup-99]

@Abhirup-99 I meant rewriting the stream_write_box to be a urwid.Text or so (as appropriate), and then use it in the same header_write_box as before. We wouldn't be changing the orientation or structure of the header_write_box. I believe the header_write_box's syntax or behavior shouldn't change.
Here's a snippet that might help make my suggestion clear for extending it in the stream_box_edit_view.

        self.stream_write_box = urwid.Text(u"")
        self.header_write_box = urwid.Columns(
            [self.stream_write_box, self.title_write_box],
            dividechars=1
        )

I got your point on this part, let me try adding a helper function for this change then because even with this approach, there would be code duplication.

[Abhirup-99]

@prah23 take a look now

[prah23]

@Abhirup-99 It looks well segregated 👍. It seems a little more complicated now though, but I got your point about prioritizing non-redundant code. I'm probably still biased towards the stream_box_edit_view extension approach because though it needed registering the stream box again, it seemed simpler to understand for me. We can definitely wait for Neil's opinion, I just wanted to drop in some suggestions, in case they helped.

Also, in your current approach, we wouldn't be needing the is_in_edit boolean, would we? Considering that you have separate views now.

[Abhirup-99]

@Abhirup-99 It looks well segregated . It seems a little more complicated now though, but I got your point about prioritizing non-redundant code. I'm probably still biased towards the stream_box_edit_view extension approach because though it needed registering the stream box again, it seemed simpler to understand for me. We can definitely wait for Neil's opinion, I just wanted to drop in some suggestions, in case they helped.

Also, in your current approach, we wouldn't be needing the is_in_edit boolean, would we? Considering that you have separate views now.

Yeah it's required while cycling through the tabs.

[Ezio-Sarthak]

Thanks for working on this! The UI seems to be on the right path!

However, there is a scope for improving/simplifying the code logic. Leaving some quick comments below :)

[neiljp]

I see there has been discussion about this - how ready is this?

[Abhirup-99]

@neiljp this pr is ready for review. I have removed all conflicts.

[zulipbot]

Heads up @Abhirup-99, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.

[neiljp]

This code is merged after simplification as #1161, which was itself merged manually.