-
Notifications
You must be signed in to change notification settings - Fork 35
Added a Stop/Close Button to sidebar to solve Issue #151 #181
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request introduces a new function Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as Sidebar Button
participant A as close_app()
participant OS as Operating System
participant P as Process Manager
U->>S: Click "Stop/Close"
S->>A: Trigger close_app()
A->>A: Display shutdown spinner
A->>OS: Execute keyboard shortcut to close tab
alt Tab closes successfully
A->>P: Terminate Streamlit process using psutil
else Tab fails to close
A->>U: Display warning message (macOS instructions)
A->>P: Terminate Streamlit process using psutil
end
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app.py(2 hunks)requirements.txt(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
app.py
59-59: Local variable error is assigned to but never used
Remove assignment to unused variable error
(F841)
🔇 Additional comments (4)
requirements.txt (1)
12-12: PyAutoGUI dependency added for application close functionality.The addition of
pyautogui==0.9.54is appropriate for the new close application functionality, where it's used to simulate keyboard shortcuts to close browser tabs.app.py (3)
6-10: LGTM: New imports added for app termination functionality.The added imports are necessary for the new close application feature:
osfor process ID retrievaltimefor spinner delaypyautoguifor browser tab closing via keyboard shortcutspsutilfor process terminationplatformfor OS detection
13-14: LGTM: Indentation fixed for settings loading.The indentation correction ensures proper code structure when loading settings from the JSON file.
72-76: LGTM: Sidebar close button implementation.The "Stop/Close" button in the sidebar with appropriate messaging meets the requirements specified in the PR objective. The button triggers the
close_app()function to terminate the application.
| def close_app(): | ||
| """ | ||
| Closes the Streamlit app by terminating the Python process and | ||
| attempting to close the browser tab with keystrokes. | ||
| """ | ||
| with st.spinner("Shutting down..."): | ||
| time.sleep(3) # give the user a small window to see the spinner | ||
|
|
||
| # Attempt to close the current browser tab (keystroke-based) | ||
| try: | ||
| if platform.system() == "Darwin": | ||
| # macOS typically uses 'command + w' | ||
| pyautogui.hotkey('command', 'w') | ||
| else: | ||
| # Windows/Linux typically use 'ctrl + w' | ||
| pyautogui.hotkey('ctrl', 'w') | ||
| except Exception as error: | ||
| st.warning( | ||
| "We tried closing the browser window, but failed. " | ||
| "You may need to close it manually. For macOS, ensure that:" | ||
| " System Preferences → Security & Privacy → Accessibility → Terminal is checked." | ||
| ) | ||
|
|
||
| # Terminate the Streamlit python process | ||
| pid = os.getpid() | ||
| p = psutil.Process(pid) | ||
| p.terminate() | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolve unused exception variable and consider environment compatibility.
The close_app() function implementation generally looks good, but there are a couple of issues to address:
- The
errorvariable in the exception handler is assigned but never used. - PyAutoGUI's keyboard shortcuts might not work in all environments, especially in headless servers or containerized deployments.
Fix the unused variable issue:
- except Exception as error:
+ except Exception:Additionally, consider adding a comment about the environment limitations:
"""
Closes the Streamlit app by terminating the Python process and
attempting to close the browser tab with keystrokes.
+
+ Note: Tab closing via keyboard shortcuts requires a GUI environment
+ and may not work in headless servers or containerized deployments.
"""📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def close_app(): | |
| """ | |
| Closes the Streamlit app by terminating the Python process and | |
| attempting to close the browser tab with keystrokes. | |
| """ | |
| with st.spinner("Shutting down..."): | |
| time.sleep(3) # give the user a small window to see the spinner | |
| # Attempt to close the current browser tab (keystroke-based) | |
| try: | |
| if platform.system() == "Darwin": | |
| # macOS typically uses 'command + w' | |
| pyautogui.hotkey('command', 'w') | |
| else: | |
| # Windows/Linux typically use 'ctrl + w' | |
| pyautogui.hotkey('ctrl', 'w') | |
| except Exception as error: | |
| st.warning( | |
| "We tried closing the browser window, but failed. " | |
| "You may need to close it manually. For macOS, ensure that:" | |
| " System Preferences → Security & Privacy → Accessibility → Terminal is checked." | |
| ) | |
| # Terminate the Streamlit python process | |
| pid = os.getpid() | |
| p = psutil.Process(pid) | |
| p.terminate() | |
| def close_app(): | |
| """ | |
| Closes the Streamlit app by terminating the Python process and | |
| attempting to close the browser tab with keystrokes. | |
| Note: Tab closing via keyboard shortcuts requires a GUI environment | |
| and may not work in headless servers or containerized deployments. | |
| """ | |
| with st.spinner("Shutting down..."): | |
| time.sleep(3) # give the user a small window to see the spinner | |
| # Attempt to close the current browser tab (keystroke-based) | |
| try: | |
| if platform.system() == "Darwin": | |
| # macOS typically uses 'command + w' | |
| pyautogui.hotkey('command', 'w') | |
| else: | |
| # Windows/Linux typically use 'ctrl + w' | |
| pyautogui.hotkey('ctrl', 'w') | |
| except Exception: | |
| st.warning( | |
| "We tried closing the browser window, but failed. " | |
| "You may need to close it manually. For macOS, ensure that:" | |
| " System Preferences → Security & Privacy → Accessibility → Terminal is checked." | |
| ) | |
| # Terminate the Streamlit python process | |
| pid = os.getpid() | |
| p = psutil.Process(pid) | |
| p.terminate() |
🧰 Tools
🪛 Ruff (0.8.2)
59-59: Local variable error is assigned to but never used
Remove assignment to unused variable error
(F841)
| streamlit-js-eval | ||
| psutil==7.0.0 | ||
| psutil==7.0.0 | ||
| pyautogui==0.9.54 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you add for docker? https://github.com/OpenMS/streamlit-template/blob/main/environment.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done sir
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
environment.yml (1)
18-19: New Dependency Addition and File Formatting IssueThe additions of
psutil==7.0.0andpyautogui==0.9.54align with the PR’s objective of enhancing functionality for the Stop/Close button. However, please add a newline character at the end of the file to comply with YAML lint standards.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 19-19: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
environment.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
environment.yml
[error] 19-19: no new line character at the end of file
(new-line-at-end-of-file)
Hey @Arslan-Siraj
This pr solves issue (#151) using a stop/close button.
This is not automated, but it follows the suggestion of using approach made by MassDash.
Summary by CodeRabbit
New Features
Chores
pyautogui.