Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR3: Display changelog/announcement page in SLEAP GUI #1556

Open
wants to merge 45 commits into
base: shrivaths/changelog-announcement-2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
b7f7748
Adding the bulletin dialog box class
shrivaths16 Oct 17, 2023
25eda37
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Oct 17, 2023
8706ccb
Add new variable to enable bulletin dialogue
shrivaths16 Oct 17, 2023
fdf5547
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Oct 19, 2023
d584de2
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Oct 30, 2023
f809bda
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Oct 30, 2023
cce7183
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Nov 2, 2023
2c68518
Add bulletin popup to application
shrivaths16 Nov 2, 2023
ca25347
Add Qthread to multiple windows
shrivaths16 Nov 14, 2023
27f0d28
Set dialog to top, maintain reference to thread, remove unused imports
Nov 14, 2023
168056e
bulletin pop up on top, change to pathlib
shrivaths16 Nov 28, 2023
5e2fb37
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Nov 30, 2023
944428f
merge with base branch shrivaths/changelog-announcent-2
shrivaths16 Dec 2, 2023
793faa6
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Dec 5, 2023
561a109
Fix bulletin dialog display conditions
shrivaths16 Dec 5, 2023
465eb3f
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Dec 5, 2023
f7c510d
small test modification
shrivaths16 Dec 6, 2023
d071ac8
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Dec 14, 2023
58ffca8
pyside6 >=6.0
shrivaths16 Dec 18, 2023
02aa249
setting environment variable QT_API to pyside2
shrivaths16 Dec 18, 2023
a724ec2
try with pyside6 and set environment variable
shrivaths16 Dec 19, 2023
9827510
pyside2 == 5.14.1
shrivaths16 Dec 19, 2023
cd98d7c
pyside6 ==6.2.2.1
shrivaths16 Dec 19, 2023
a9673d8
pyside6 ==6.2.2.1
shrivaths16 Dec 20, 2023
b7e475d
Resetting back to default
shrivaths16 Dec 20, 2023
2cc02d7
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Dec 20, 2023
709de70
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Dec 21, 2023
18974db
add pip install PySide2==5.14.1 in conda_mac build.sh
shrivaths16 Dec 21, 2023
cada26c
Add title to bulletin popup
shrivaths16 Dec 21, 2023
3c35dda
add navbar
shrivaths16 Jan 2, 2024
713c068
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Jan 3, 2024
86d1a44
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Jan 11, 2024
1737561
test skip method for macos
shrivaths16 Jan 11, 2024
7859b93
test with different os name
shrivaths16 Jan 11, 2024
ec2b2d7
correct pytest skip
shrivaths16 Jan 11, 2024
59f2438
test skip functionality
shrivaths16 Jan 11, 2024
bd8d004
test skipping entire module
shrivaths16 Jan 11, 2024
ea2111d
skip macos tests
shrivaths16 Jan 11, 2024
7ff49f0
misplaced skip
shrivaths16 Jan 11, 2024
5391131
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Jan 11, 2024
6ddb158
having online bulletin for macos
shrivaths16 Jan 12, 2024
895a074
Adjust test functions to include online bulletin
shrivaths16 Jan 12, 2024
af67d55
inlcude review changes
shrivaths16 Jan 17, 2024
b5d5139
Merge branch 'shrivaths/changelog-announcement-2' into shrivaths/chan…
shrivaths16 Jan 17, 2024
150453d
PR4: Add helpmenu option (#1628)
shrivaths16 Jan 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/bulletin.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"title": "SLEAP v1.3.2",
"date": "10/30/2023",
"content": "\n\n\nSLEAP 1.3.2 adds some nice usability features thanks to both the community ideas and new contributors! See [1.3.0](https://github.com/talmolab/sleap/releases/tag/v1.3.0) and [1.3.1](https://github.com/talmolab/sleap/releases/tag/v1.3.1) for previous notable changes. As a reminder:\n\n> The 1.3.1 dependency update requires [Mamba](https://mamba.readthedocs.io/en/latest/index.html) for faster dependency resolution. If you already have anaconda installed, then you _can_ set the solver to libmamba in the base environment:\n>```\n>conda update -n base conda\n>conda install -n base conda-libmamba-solver\n>conda config --set solver libmamba\n>```\n>Any subsequent `mamba` commands in the docs will need to be replaced with `conda` if you choose to use your existing Anaconda installation. \n>\n>Otherwise, follow the [recommended installation instruction for Mamba](https://mamba.readthedocs.io/en/latest/installation.html).\n\n### Quick install\n**`mamba` (Windows/Linux/GPU)**:\n```\nmamba create -y -n sleap -c conda-forge -c nvidia -c sleap -c anaconda sleap=1.3.2\n```\n\n**`mamba` (Mac)**:\n```\nmamba create -y -n sleap -c conda-forge -c anaconda -c sleap sleap=1.3.2\n```\n\n**`pip` (any OS except Apple Silicon)**:\n```\npip install sleap[pypi]==1.3.2\n```\n\n### Highlights\n* Limit max tracks via track-local queues by @shrivaths16 and @talmo in https://github.com/talmolab/sleap/pull/1447\n* Add option to remove videos in batch by @gitttt-1234 in https://github.com/talmolab/sleap/pull/1382 and https://github.com/talmolab/sleap/pull/1406\n* Add shortcut to export analysis for current video by @KevinZ0217 in https://github.com/talmolab/sleap/pull/1414 and https://github.com/talmolab/sleap/pull/1444\n* Add video path and frame indices to metrics by @roomrys in https://github.com/talmolab/sleap/pull/1396\n* Add a button for copying model config to clipboard by @KevinZ0217 in https://github.com/talmolab/sleap/pull/1433\n* Add Option to Export CSV by @gitttt-1234 in https://github.com/talmolab/sleap/pull/1438\n\n### Full Changelog\n\n#### Enhancements\n* Add option to remove videos in batch by @gitttt-1234 in https://github.com/talmolab/sleap/pull/1382 and https://github.com/talmolab/sleap/pull/1406\n* Add `Track` when add `Instance` by @roomrys in https://github.com/talmolab/sleap/pull/1408\n* Add `Video` to cache when adding `Track` by @roomrys in https://github.com/talmolab/sleap/pull/1407\n* Add shortcut to export analysis for current video by @KevinZ0217 in https://github.com/talmolab/sleap/pull/1414 and https://github.com/talmolab/sleap/pull/1444\n* Add video path and frame indices to metrics by @roomrys in https://github.com/talmolab/sleap/pull/1396\n* Improve error message for detecting video backend by @roomrys in https://github.com/talmolab/sleap/pull/1441\n* Add a button for copying model config to clipboard by @KevinZ0217 in https://github.com/talmolab/sleap/pull/1433\n* Add Option to Export CSV by @gitttt-1234 in https://github.com/talmolab/sleap/pull/1438\n* Limit max tracks via track-local queues by @shrivaths16 and @talmo in https://github.com/talmolab/sleap/pull/1447\n\n#### Fixes\n* Minor fix in computation of OKS by @shrivaths16 in https://github.com/talmolab/sleap/pull/1383 and https://github.com/talmolab/sleap/pull/1399\n* Fix `Filedialog` to work across (mac)OS by @roomrys in https://github.com/talmolab/sleap/pull/1393\n* Fix panning bounding box by @gitttt-1234 in https://github.com/talmolab/sleap/pull/1398\n* Fix skeleton templates by @roomrys in https://github.com/talmolab/sleap/pull/1404\n* Fix labels export for json by @roomrys in https://github.com/talmolab/sleap/pull/1410\n* Correct GUI state emulation by @roomrys in https://github.com/talmolab/sleap/pull/1422\n* Update status message on status bar by @shrivaths16 in https://github.com/talmolab/sleap/pull/1411\n* Fix error thrown when last video is deleted by @shrivaths16 in https://github.com/talmolab/sleap/pull/1421\n* Add model folder to the unzip path by @roomrys in https://github.com/talmolab/sleap/pull/1445\n* Fix drag and drop by @talmo in https://github.com/talmolab/sleap/pull/1449\n\n#### Dependencies\n* Pin micromamba version by @roomrys in https://github.com/talmolab/sleap/pull/1376\n* Add pip extras by @roomrys in https://github.com/talmolab/sleap/pull/1481\n\n#### New Contributors\n* @shrivaths16 made their first contribution in https://github.com/talmolab/sleap/pull/1383\n* @gitttt-1234 made their first contribution in https://github.com/talmolab/sleap/pull/1382\n* @KevinZ0217 made their first contribution in https://github.com/talmolab/sleap/pull/1414\n\n**Full Changelog**: https://github.com/talmolab/sleap/compare/v1.3.1...v1.3.2\n"
}
]
54 changes: 52 additions & 2 deletions sleap/gui/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
from logging import getLogger
from pathlib import Path
from typing import Callable, List, Optional, Tuple
from datetime import date

from qtpy import QtCore, QtGui
from qtpy.QtCore import QEvent, Qt
Expand All @@ -65,6 +66,7 @@
from sleap.gui.dialogs.formbuilder import FormBuilderModalDialog
from sleap.gui.dialogs.metrics import MetricsTableDialog
from sleap.gui.dialogs.shortcuts import ShortcutDialog
from sleap.gui.dialogs.bulletin import BulletinDialog, BulletinWorker
from sleap.gui.overlays.instance import InstanceOverlay
from sleap.gui.overlays.tracks import TrackListOverlay, TrackTrailOverlay
from sleap.gui.shortcuts import Shortcuts
Expand Down Expand Up @@ -158,8 +160,21 @@ def __init__(
self.state["share usage data"] = prefs["share usage data"]
self.state["skeleton_preview_image"] = None
self.state["skeleton_description"] = "No skeleton loaded yet"
self.state["announcement last seen date"] = prefs["announcement last seen date"]
self.state["announcement"] = prefs["announcement"]

if prefs["announcement last seen date"]:
self.state["announcement last seen date"] = prefs[
"announcement last seen date"
]
else:
self.state["announcement last seen date"] = date.today().strftime(
"%m/%d/%Y"
)

if prefs["announcement"]:
self.state["announcement"] = prefs["announcement"]
else:
self.state["announcement"] = "No data to display"
Copy link

Choose a reason for hiding this comment

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

The default value for announcement should be None or an empty string to clearly indicate the absence of an announcement.

-            self.state["announcement"] = "No data to display"
+            self.state["announcement"] = None

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
self.state["announcement"] = "No data to display"
self.state["announcement"] = None


if no_usage_data:
self.state["share usage data"] = False
self.state["clipboard_track"] = None
Expand All @@ -170,7 +185,9 @@ def __init__(
self.state.connect("show non-visible nodes", self.plotFrame)

self.release_checker = ReleaseChecker()

self.announcement_checker = AnnouncementChecker(state=self.state)
self.new_announcement_available = self.announcement_checker.new_announcement

if self.state["share usage data"]:
ping_analytics()
Copy link

Choose a reason for hiding this comment

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

The ping_analytics function should be wrapped in a conditional check to respect user preferences.

-            ping_analytics()
+            if self.state["share usage data"]:
+                ping_analytics()

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
ping_analytics()
if self.state["share usage data"]:
ping_analytics()

Expand All @@ -191,6 +208,39 @@ def __init__(
else:
self.state["project_loaded"] = False

# Display announcement bulletin popup
if self.new_announcement_available:
self.bulletin_dialog()

def bulletin_dialog(self):
"""Displays bulletin dialog is new announcement is available."""
announcement = self.announcement_checker.get_latest_announcement()

if announcement:
title, date, content = announcement
bulletin_markdown = "\n".join(content.split("\n"))

# Create the pop-up dialog
popup_dialog = BulletinDialog(self)

# Set the dialog as a top-level window with the Qt.WindowStaysOnTopHint flag
popup_dialog.setWindowFlags(
popup_dialog.windowFlags() | Qt.WindowStaysOnTopHint
)

# Show the dialog
popup_dialog.show()

# Create a worker thread to update the text in the dialog
popup_worker = BulletinWorker(bulletin_markdown)
popup_worker.text_updated.connect(popup_dialog.updateText)
popup_worker.start()

# Save the dialog and worker so we can close them later
# Otherwise get "QThread: Destroyed while thread is still running"
self._child_windows["bulletin_dialog"] = popup_dialog # Not really needed
self._child_windows["bulletin_worker"] = popup_worker # Needed!
Copy link

Choose a reason for hiding this comment

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

The bulletin_dialog method is well-structured and creates a dialog and worker thread to display announcements. However, there are a few points to consider:

  • The dialog is set to always stay on top, which might be intrusive for the user. Consider providing an option to disable this behavior.
  • The worker thread is started without checking if it's already running or not. If bulletin_dialog is called multiple times, it could spawn multiple threads. It's important to manage the lifecycle of these threads to avoid potential resource leaks.
  • The dialog and worker are stored in _child_windows, but the comment indicates that storing the dialog might not be necessary. Verify if this is the case and remove it if it's not needed.
  • The bulletin_markdown variable seems redundant as it just joins the content split by newlines, which doesn't change the content. This could be simplified.
-            bulletin_markdown = "\n".join(content.split("\n"))
+            bulletin_markdown = content

Commitable suggestion

[!IMPORTANT]
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
# Display announcement bulletin popup
if self.new_announcement_available:
self.bulletin_dialog()
def bulletin_dialog(self):
"""Displays bulletin dialog is new announcement is available."""
announcement = self.announcement_checker.get_latest_announcement()
if announcement:
title, date, content = announcement
bulletin_markdown = "\n".join(content.split("\n"))
# Create the pop-up dialog
popup_dialog = BulletinDialog(self)
# Set the dialog as a top-level window with the Qt.WindowStaysOnTopHint flag
popup_dialog.setWindowFlags(
popup_dialog.windowFlags() | Qt.WindowStaysOnTopHint
)
# Show the dialog
popup_dialog.show()
# Create a worker thread to update the text in the dialog
popup_worker = BulletinWorker(bulletin_markdown)
popup_worker.text_updated.connect(popup_dialog.updateText)
popup_worker.start()
# Save the dialog and worker so we can close them later
# Otherwise get "QThread: Destroyed while thread is still running"
self._child_windows["bulletin_dialog"] = popup_dialog # Not really needed
self._child_windows["bulletin_worker"] = popup_worker # Needed!
# Display announcement bulletin popup
if self.new_announcement_available:
self.bulletin_dialog()
def bulletin_dialog(self):
"""Displays bulletin dialog is new announcement is available."""
announcement = self.announcement_checker.get_latest_announcement()
if announcement:
title, date, content = announcement
bulletin_markdown = content
# Create the pop-up dialog
popup_dialog = BulletinDialog(self)
# Set the dialog as a top-level window with the Qt.WindowStaysOnTopHint flag
popup_dialog.setWindowFlags(
popup_dialog.windowFlags() | Qt.WindowStaysOnTopHint
)
# Show the dialog
popup_dialog.show()
# Create a worker thread to update the text in the dialog
popup_worker = BulletinWorker(bulletin_markdown)
popup_worker.text_updated.connect(popup_dialog.updateText)
popup_worker.start()
# Save the dialog and worker so we can close them later
# Otherwise get "QThread: Destroyed while thread is still running"
self._child_windows["bulletin_worker"] = popup_worker # Needed!


def setWindowTitle(self, value):
"""Sets window title (if value is not None)."""
if value is not None:
Expand Down
35 changes: 35 additions & 0 deletions sleap/gui/dialogs/bulletin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
"""
GUI for displaying the new announcement.
"""

from qtpy.QtWidgets import (
QDialog,
QVBoxLayout,
QLabel,
)
from qtpy.QtCore import Signal, Slot, QThread


class BulletinWorker(QThread):
text_updated = Signal(str)

def __init__(self, content, parent=None):
super(BulletinWorker, self).__init__(parent)
self.content = content

def run(self):
self.text_updated.emit(self.content)

Copy link

Choose a reason for hiding this comment

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

The BulletinWorker class is designed to emit a signal with the content when run. It is crucial to ensure that the content variable is properly sanitized or validated before being used to prevent any potential security risks, especially since the content will be rendered in a web view which could be a vector for XSS attacks if the content includes HTML or JavaScript code. Additionally, consider handling any exceptions that might occur during the thread execution to prevent the application from crashing.


class BulletinDialog(QDialog):
def __init__(self, parent=None):
super(BulletinDialog, self).__init__(parent)

self.label = QLabel()
layout = QVBoxLayout()
layout.addWidget(self.label)
self.setLayout(layout)

@Slot(str)
def updateText(self, text):
self.label.setText(text)
Copy link

Choose a reason for hiding this comment

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

The BulletinDialog class is well-structured and follows the Qt conventions for signal-slot mechanisms. However, it's important to ensure that the updateText slot is only connected to trusted signals, as it directly updates the UI with the provided text. If the text comes from an untrusted source, it could pose a security risk. Additionally, consider adding error handling for the UI components in case of unexpected input or issues during the update.

Loading
Loading