Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 1 addition & 7 deletions src/news/__init__.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
from ._newswidget import NewsWidget
from .newsitem import NewsItem
from .newsmanager import NewsManager
from .wpapi import WPAPI

__all__ = (
"NewsWidget",
"NewsItem",
"NewsManager",
"WPAPI",
)
)
156 changes: 18 additions & 138 deletions src/news/_newswidget.py
Original file line number Diff line number Diff line change
@@ -1,155 +1,35 @@
import logging
import os.path
from typing import Any

from PyQt6.QtCore import QPoint
from PyQt6.QtCore import QSize
from PyQt6.QtCore import QUrl
from PyQt6.QtGui import QDesktopServices
from PyQt6.QtGui import QPixmap
from PyQt6.QtWidgets import QToolTip
from PyQt6.QtWidgets import QWidget
from PyQt5.QtCore import QUrl
from PyQt5.QtWidgets import QWidget, QVBoxLayout
from PyQt5.QtWebEngineWidgets import QWebEngineView
Comment on lines +2 to +4

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify declared Qt dependencies and binding consistency.
fd -t f 'requirements.*\.txt|pyproject\.toml|setup\.py|setup\.cfg|Pipfile' \
  -0 | xargs -0 -r rg -n -i 'pyqt|webengine'

rg -n --type py -C2 'from PyQt[56]|\.exec_\s*\(' \
  src test_web_isolated.py

Repository: FAForever/client

Length of output: 50372


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and inspect the news widget and the isolated test.
git ls-files 'src/news/_newswidget.py' 'test_web_isolated.py'
printf '\n---\n'
for f in src/news/_newswidget.py test_web_isolated.py; do
  if [ -f "$f" ]; then
    echo "FILE: $f"
    cat -n "$f" | sed -n '1,80p'
    echo '---'
  fi
done

Repository: FAForever/client

Length of output: 2885


Use PyQt6 throughout the news widget and isolated test

src/news/_newswidget.py and test_web_isolated.py still import PyQt5, and the test harness also uses app.exec_(). Switch both files to PyQt6 and app.exec(). If QWebEngineView stays, add the matching PyQt6 WebEngine dependency.

📍 Affects 2 files
  • src/news/_newswidget.py#L2-L4 (this comment)
  • test_web_isolated.py#L2-L4
  • test_web_isolated.py#L25-L28
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/news/_newswidget.py` around lines 2 - 4, Migrate the news widget and
isolated test from PyQt5 to PyQt6: update imports in src/news/_newswidget.py
(lines 2-4) and test_web_isolated.py (lines 2-4), replace app.exec_() with
app.exec() in test_web_isolated.py (lines 25-28), and add the corresponding
PyQt6 WebEngine dependency if QWebEngineView remains in use.


from src import util
from src.config import Settings
from src.downloadManager import Downloader
from src.downloadManager import DownloadRequest

from .newsitem import NewsItem
from .newsitem import NewsItemDelegate
from .newsmanager import NewsManager

logger = logging.getLogger(__name__)


# Safely load the default container UI
FormClass, BaseClass = util.THEME.loadUiType("news/news.ui")


class NewsWidget(FormClass, BaseClass):
IMAGE_SIZE = QSize(600, 338)
NEWS_FEED_URL = "https://faforever.github.io/FAF-Newsfeed/"

def __init__(self, parent: QWidget | None = None) -> None:
BaseClass.__init__(self, parent)

self.setupUi(self)

self._downloader = Downloader(util.NEWS_CACHE_DIR)
self._images_dl_request = DownloadRequest()
self._images_dl_request.done.connect(self.item_image_downloaded)

self.newsManager = NewsManager(self)
self.newsItems: list[NewsItem] = []

self.settingsFrame.hide()
self.hideNewsEdit.setText(Settings.get('news/hideWords', ""))

self.newsList.setIconSize(QSize(0, 0))
self.newsList.setItemDelegate(NewsItemDelegate(self))
self.newsList.currentItemChanged.connect(self.itemChanged)
self.newsSettings.pressed.connect(self.showSettings)
self.showAllButton.pressed.connect(self.showAll)
self.hideNewsEdit.textEdited.connect(self.updateNewsFilter)
self.hideNewsEdit.cursorPositionChanged.connect(self.showEditToolTip)
self.newsLinkButton.clicked.connect(self.open_news_in_browser)

def addNews(self, newsPost: dict[str, Any]) -> None:
newsItem = NewsItem(newsPost, self.newsList)
self.newsItems.append(newsItem)

def download_image(self, img_url: str) -> None:
name = os.path.basename(img_url)
self._downloader.download(name, self._images_dl_request, img_url)

def item_image_downloaded(self, image_name: str, result: tuple[str, bool]) -> None:
image_path, download_failed = result
if not download_failed:
pixmap = QPixmap(image_path)
scaled = pixmap.scaled(self.IMAGE_SIZE)
self.imageLabel.setPixmap(scaled)
# Check if news.ui already defines a "newsLayout", otherwise build one
if hasattr(self, 'newsLayout') and self.newsLayout is not None:
layout = self.newsLayout
else:
self.imageLabel.clear()
self.show_newspage()

def itemChanged(self, current: NewsItem | None, previous: NewsItem | None) -> None:
if current is None:
return
layout = QVBoxLayout(self)
self.setLayout(layout)

url = current.newsPost["img_url"]
image_name = os.path.basename(url)
image_path = os.path.join(util.NEWS_CACHE_DIR, image_name)
if os.path.isfile(image_path):
self.imageLabel.setPixmap(QPixmap(image_path).scaled(self.IMAGE_SIZE))
self.show_newspage()
else:
self.imageLabel.clear()
self._downloader.download(image_name, self._images_dl_request, url)

def show_newspage(self) -> None:
current = self.newsList.currentItem()
if current is None:
return
content = current.newsPost["excerpt"].strip().removeprefix("<p>").removesuffix("</p>")
self.newsTitleLabel.setText(current.newsPost["title"])
self.bodyLabel.setText(content)

def showAll(self) -> None:
for item in self.newsItems:
item.setHidden(False)
self.updateLabel(0)

def showEditToolTip(self) -> None:
"""
Default tooltips are too slow and disappear when user starts typing
"""
widget = self.hideNewsEdit
position = widget.mapToGlobal(
QPoint(int(widget.width()), -widget.height() // 2),
)
QToolTip.showText(
position,
"To separate multiple words use commas: nomads,server,dev",
)

def showSettings(self):
if self.settingsFrame.isHidden():
self.settingsFrame.show()
else:
self.settingsFrame.hide()

def updateNewsFilter(self, text=False):
if text is not False:
Settings.set('news/hideWords', text)

filterList = Settings.get('news/hideWords', "").lower().split(",")
newsHidden = 0

if filterList[0]:
for item in self.newsItems:
for word in filterList:
if word in item.newsPost["title"].lower():
item.setHidden(True)
newsHidden += 1
break
else:
item.setHidden(False)
else:
for item in self.newsItems:
item.setHidden(False)

self.updateLabel(newsHidden)

def updateLabel(self, number):
self.totalHidden.setText("NEWS HIDDEN: " + str(number))

def open_news_in_browser(self) -> None:
current = self.newsList.currentItem()
if current is None:
return
if current.newsPost["external_link"] == "":
external_link = current.newsPost["link"]
else:
external_link = current.newsPost["external_link"]
QDesktopServices.openUrl(QUrl(external_link))
# Inject the new WebEngine client
self.browser = QWebEngineView(self)
self.browser.setUrl(QUrl(self.NEWS_FEED_URL))

layout.addWidget(self.browser)

def on_news_loaded(self) -> None:
self.stackedWidget.setCurrentIndex(1)
# Keep this trigger alive for the parent manager navigation flow
if hasattr(self, 'stackedWidget') and self.stackedWidget is not None:
self.stackedWidget.setCurrentIndex(1)
Comment on lines +27 to +35

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Connect page completion to the stacked-widget transition.

The new browser never invokes on_news_loaded, so a loading page in stackedWidget will remain selected. Connect loadFinished before navigation and avoid revealing a blank browser when loading fails.

Proposed fix
         self.browser = QWebEngineView(self)
+        self.browser.loadFinished.connect(self.on_news_loaded)
         self.browser.setUrl(QUrl(self.NEWS_FEED_URL))
 
         layout.addWidget(self.browser)
 
-    def on_news_loaded(self) -> None:
+    def on_news_loaded(self, success: bool) -> None:
+        if not success:
+            logger.warning("Failed to load news feed")
+            return
         if hasattr(self, 'stackedWidget') and self.stackedWidget is not None:
             self.stackedWidget.setCurrentIndex(1)
📝 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.

Suggested change
self.browser = QWebEngineView(self)
self.browser.setUrl(QUrl(self.NEWS_FEED_URL))
layout.addWidget(self.browser)
def on_news_loaded(self) -> None:
self.stackedWidget.setCurrentIndex(1)
# Keep this trigger alive for the parent manager navigation flow
if hasattr(self, 'stackedWidget') and self.stackedWidget is not None:
self.stackedWidget.setCurrentIndex(1)
self.browser = QWebEngineView(self)
self.browser.loadFinished.connect(self.on_news_loaded)
self.browser.setUrl(QUrl(self.NEWS_FEED_URL))
layout.addWidget(self.browser)
def on_news_loaded(self, success: bool) -> None:
if not success:
logger.warning("Failed to load news feed")
return
# Keep this trigger alive for the parent manager navigation flow
if hasattr(self, 'stackedWidget') and self.stackedWidget is not None:
self.stackedWidget.setCurrentIndex(1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/news/_newswidget.py` around lines 27 - 35, Update the browser setup in
the widget initializer to connect QWebEngineView.loadFinished to on_news_loaded
before calling setUrl, and update on_news_loaded to transition stackedWidget
only when the load succeeds. Preserve the existing stackedWidget guards and keep
the loading page selected when navigation fails.

92 changes: 0 additions & 92 deletions src/news/newsitem.py

This file was deleted.

35 changes: 0 additions & 35 deletions src/news/newsmanager.py

This file was deleted.

49 changes: 0 additions & 49 deletions src/news/wpapi.py

This file was deleted.

28 changes: 28 additions & 0 deletions test_web_isolated.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import sys
from PyQt5.QtCore import QUrl
from PyQt5.QtWidgets import QApplication, QMainWindow, QVBoxLayout, QWidget
from PyQt5.QtWebEngineWidgets import QWebEngineView

class StandaloneNewsWindow(QMainWindow):
def __init__(self):
super().__init__()
self.setWindowTitle("FAF Newsfeed - Web-Only Test Harness")
self.resize(1024, 768)

# Main Layout container
central_widget = QWidget(self)
self.setCentralWidget(central_widget)
layout = QVBoxLayout(central_widget)
layout.setContentsMargins(0, 0, 0, 0)

# Embedded Chromium browser instance
self.browser = QWebEngineView(self)
self.browser.setUrl(QUrl("https://faforever.github.io/FAF-Newsfeed/"))

layout.addWidget(self.browser)

if __name__ == "__main__":
app = QApplication(sys.argv)
window = StandaloneNewsWindow()
window.show()
sys.exit(app.exec_())
Loading