Skip to content

Conversation

@teamcons
Copy link
Contributor

@teamcons teamcons commented Apr 5, 2025

Review Checklist

  • App opens
  • Does what it says
  • Categories match

AppData

  • Name is unique and non-confusing
  • Matches description
  • Matches screenshot
  • Launchable tag with matching ID
  • Release tag with matching version and YYYY-MM-DD date
  • OARS info matches

Flatpak

  • Uses elementary runtime
  • Sandbox permissions are reasonable

@teamcons teamcons requested a review from a team as a code owner April 5, 2025 17:46
@ryonakano
Copy link
Member

@teamcons
Copy link
Contributor Author

teamcons commented Apr 6, 2025

No. Oversight.

@ryonakano
Copy link
Member

Just out of curiosity (I don't mean to block your release), but is there any reason why you removed the close button in the headerbar? I personally thought now it looks as if there is no visual widgets to save notes and close the app, while it's still possible with Ctrl+Q, Alt+F4, "Close" menuitem in the context menu of the headerbar, etc. though.

image

Apparently you looks like changed this intentionally in 2.4.0:

[ryo@b760m ~/work/tmp/Jorts (main =)]$ git diff 2.3.0..2.4.0 -- src
:

diff --git a/src/MainWindow.vala b/src/MainWindow.vala
index 7360de7..80d5b1e 100644
--- a/src/MainWindow.vala
+++ b/src/MainWindow.vala
:

             /**********************************************/
             /*              USER INTERFACE                */
             /**********************************************/

-            var headerbar = new Gtk.HeaderBar();
+            this.headerbar = new Gtk.HeaderBar();
             headerbar.add_css_class ("flat");
             headerbar.add_css_class("headertitle");
             //header.has_subtitle = false;
-            headerbar.set_show_title_buttons (true);
-            headerbar.decoration_layout = "close:";
+
+            //headerbar.decoration_layout = "close:";
+            headerbar.set_show_title_buttons(false);

@ryonakano ryonakano changed the title Update Jorts to 2.4.0 Update Jorts to 2.4.1 Apr 6, 2025
@teamcons
Copy link
Contributor Author

teamcons commented Apr 6, 2025

Just out of curiosity (I don't mean to block your release), but is there any reason why you removed the close button in the headerbar? I personally thought now it looks as if there is no visual widgets to save notes and close the app, while it's still possible with Ctrl+Q, Alt+F4, "Close" menuitem in the context menu of the headerbar, etc. though.

There was a lot of back and forth on my side about this, with two competing approaches:
-provide a way to reopen all notes, including closed ones, with a desktop action
-follow through on the minimalist approach: the idea was that if you do not need a note anymore, you simply delete it.

i went with the latter - it does not really make sense to manage notes between closed but still needed and visible ones.

Should the user really need to note down something, but also keep it out of sight like until next login (when all of the storage is loaded again), there is still the right-click -> Close or the overview with all notes when you click on the icon.
The notes are saved all the same.

I didnt provide a toggle to let user choose. I would eventually be open to if people have a workflow depending on opening/closing notes, so they can have a close and desktop action for reopen all,
But id like, like eOS, to provide a one flow approach, and avoid featurecreep - not try to do too much, not overwhelm too much

@teamcons teamcons changed the title Update Jorts to 2.4.1 Update Jorts to 2.4.2 Apr 6, 2025
@ryonakano
Copy link
Member

Thanks for clarification. Users might want to close a private note without deleting it when they want to hide it temporary, but your app already covers that case with another approach. So maybe removing the close button wouldn't be a big problem.

Anyway, I don't mean to block your release anymore as I said :) Approving!

Copy link
Member

@ryonakano ryonakano left a comment

Choose a reason for hiding this comment

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

Works confirmed with 2.4.2 and no concerned changes are found between 2.3.0 and 2.4.2

@ryonakano ryonakano merged commit f969862 into elementary:main Apr 8, 2025
4 checks passed
@teamcons
Copy link
Contributor Author

teamcons commented Apr 8, 2025

Thanks for clarification. Users might want to close a private note without deleting it when they want to hide it temporary, but your app already covers that case with another approach. So maybe removing the close button wouldn't be a big problem.

If the issue would have been avoiding screen noise, i considered a minimize button as well

so the choice may not be definitive either

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.

2 participants