From a057cbe481eb4363edce0bc9b91b5a1a13c0b393 Mon Sep 17 00:00:00 2001 From: Jaime van Kessel Date: Wed, 1 Dec 2021 10:18:04 +0100 Subject: [PATCH 1/5] Minor codestyle cleanup Boyscouting CURA-8696 --- cura/UI/WhatsNewPagesModel.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/cura/UI/WhatsNewPagesModel.py b/cura/UI/WhatsNewPagesModel.py index 11320a0ebb..1627f3e3ce 100644 --- a/cura/UI/WhatsNewPagesModel.py +++ b/cura/UI/WhatsNewPagesModel.py @@ -1,18 +1,22 @@ # Copyright (c) 2021 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -from .WelcomePagesModel import WelcomePagesModel import os from typing import Optional, Dict, List, Tuple + from PyQt5.QtCore import pyqtProperty, pyqtSlot + from UM.Logger import Logger from UM.Resources import Resources -# -# This Qt ListModel is more or less the same the WelcomePagesModel, except that this model is only for showing the -# "what's new" page. This is also used in the "Help" menu to show the changes log. -# +from cura.UI.WelcomePagesModel import WelcomePagesModel + + class WhatsNewPagesModel(WelcomePagesModel): + """ + This Qt ListModel is more or less the same the WelcomePagesModel, except that this model is only for showing the + "what's new" page. This is also used in the "Help" menu to show the changes log. + """ image_formats = [".png", ".jpg", ".jpeg", ".gif", ".svg"] text_formats = [".txt", ".htm", ".html"] @@ -21,7 +25,7 @@ class WhatsNewPagesModel(WelcomePagesModel): @staticmethod def _collectOrdinalFiles(resource_type: int, include: List[str]) -> Tuple[Dict[int, str], int]: - result = {} #type: Dict[int, str] + result = {} # type: Dict[int, str] highest = -1 try: folder_path = Resources.getPath(resource_type, "whats_new") From 6bce9453e4173a930b447c8a9b6b31faeb3c9e8c Mon Sep 17 00:00:00 2001 From: Jaime van Kessel Date: Wed, 1 Dec 2021 10:21:07 +0100 Subject: [PATCH 2/5] Fix warnings in newpages model Boyscouting. These somehow snuck through code review CURA-8696 --- cura/UI/WhatsNewPagesModel.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/cura/UI/WhatsNewPagesModel.py b/cura/UI/WhatsNewPagesModel.py index 1627f3e3ce..b99bdf30f0 100644 --- a/cura/UI/WhatsNewPagesModel.py +++ b/cura/UI/WhatsNewPagesModel.py @@ -2,7 +2,7 @@ # Cura is released under the terms of the LGPLv3 or higher. import os -from typing import Optional, Dict, List, Tuple +from typing import Optional, Dict, List, Tuple, TYPE_CHECKING from PyQt5.QtCore import pyqtProperty, pyqtSlot @@ -11,6 +11,10 @@ from UM.Resources import Resources from cura.UI.WelcomePagesModel import WelcomePagesModel +if TYPE_CHECKING: + from PyQt5.QtCore import QObject + from cura.CuraApplication import CuraApplication + class WhatsNewPagesModel(WelcomePagesModel): """ @@ -23,6 +27,10 @@ class WhatsNewPagesModel(WelcomePagesModel): image_key = "image" text_key = "text" + def __init__(self, application: "CuraApplication", parent: Optional["QObject"] = None) -> None: + super().__init__(application, parent) + self._subpages: List[Dict[str, Optional[str]]] = [] + @staticmethod def _collectOrdinalFiles(resource_type: int, include: List[str]) -> Tuple[Dict[int, str], int]: result = {} # type: Dict[int, str] @@ -69,7 +77,7 @@ class WhatsNewPagesModel(WelcomePagesModel): texts, max_text = WhatsNewPagesModel._collectOrdinalFiles(Resources.Texts, WhatsNewPagesModel.text_formats) highest = max(max_image, max_text) - self._subpages = [] #type: List[Dict[str, Optional[str]]] + self._subpages = [] for n in range(0, highest + 1): self._subpages.append({ WhatsNewPagesModel.image_key: None if n not in images else images[n], @@ -97,5 +105,3 @@ class WhatsNewPagesModel(WelcomePagesModel): def getSubpageText(self, page: int) -> str: result = self._getSubpageItem(page, WhatsNewPagesModel.text_key) return result if result else "* * *" - -__all__ = ["WhatsNewPagesModel"] From 051c3ee2e290f65803b49e8a49b0212998c36b95 Mon Sep 17 00:00:00 2001 From: Jaime van Kessel Date: Wed, 1 Dec 2021 10:29:31 +0100 Subject: [PATCH 3/5] Fix up documentation of WelcomePageModel Some more boyscouting CURA-8696 --- cura/UI/WelcomePagesModel.py | 108 ++++++++++++++++++++--------------- 1 file changed, 62 insertions(+), 46 deletions(-) diff --git a/cura/UI/WelcomePagesModel.py b/cura/UI/WelcomePagesModel.py index 3c2d0503ab..ec0958d07e 100644 --- a/cura/UI/WelcomePagesModel.py +++ b/cura/UI/WelcomePagesModel.py @@ -1,7 +1,9 @@ -# Copyright (c) 2019 Ultimaker B.V. +# Copyright (c) 2021 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -from collections import deque + import os + +from collections import deque from typing import TYPE_CHECKING, Optional, List, Dict, Any from PyQt5.QtCore import QUrl, Qt, pyqtSlot, pyqtProperty, pyqtSignal @@ -16,24 +18,23 @@ if TYPE_CHECKING: from cura.CuraApplication import CuraApplication -# -# This is the Qt ListModel that contains all welcome pages data. Each page is a page that can be shown as a step in the -# welcome wizard dialog. Each item in this ListModel represents a page, which contains the following fields: -# -# - id : A unique page_id which can be used in function goToPage(page_id) -# - page_url : The QUrl to the QML file that contains the content of this page -# - next_page_id : (OPTIONAL) The next page ID to go to when this page finished. This is optional. If this is not -# provided, it will go to the page with the current index + 1 -# - next_page_button_text: (OPTIONAL) The text to show for the "next" button, by default it's the translated text of -# "Next". Note that each step QML can decide whether to use this text or not, so it's not -# mandatory. -# - should_show_function : (OPTIONAL) An optional function that returns True/False indicating if this page should be -# shown. By default all pages should be shown. If a function returns False, that page will -# be skipped and its next page will be shown. -# -# Note that in any case, a page that has its "should_show_function" == False will ALWAYS be skipped. -# class WelcomePagesModel(ListModel): + """ + This is the Qt ListModel that contains all welcome pages data. Each page is a page that can be shown as a step in + the welcome wizard dialog. Each item in this ListModel represents a page, which contains the following fields: + - id : A unique page_id which can be used in function goToPage(page_id) + - page_url : The QUrl to the QML file that contains the content of this page + - next_page_id : (OPTIONAL) The next page ID to go to when this page finished. This is optional. If this is + not provided, it will go to the page with the current index + 1 + - next_page_button_text : (OPTIONAL) The text to show for the "next" button, by default it's the translated text of + "Next". Note that each step QML can decide whether to use this text or not, so it's not + mandatory. + - should_show_function : (OPTIONAL) An optional function that returns True/False indicating if this page should be + shown. By default all pages should be shown. If a function returns False, that page will + be skipped and its next page will be shown. + + Note that in any case, a page that has its "should_show_function" == False will ALWAYS be skipped. + """ IdRole = Qt.UserRole + 1 # Page ID PageUrlRole = Qt.UserRole + 2 # URL to the page's QML file @@ -55,11 +56,11 @@ class WelcomePagesModel(ListModel): self._default_next_button_text = self._catalog.i18nc("@action:button", "Next") - self._pages = [] # type: List[Dict[str, Any]] + self._pages: List[Dict[str, Any]] = [] self._current_page_index = 0 # Store all the previous page indices so it can go back. - self._previous_page_indices_stack = deque() # type: deque + self._previous_page_indices_stack: deque = deque() # If the welcome flow should be shown. It can show the complete flow or just the changelog depending on the # specific case. See initialize() for how this variable is set. @@ -72,17 +73,21 @@ class WelcomePagesModel(ListModel): def currentPageIndex(self) -> int: return self._current_page_index - # Returns a float number in [0, 1] which indicates the current progress. @pyqtProperty(float, notify = currentPageIndexChanged) def currentProgress(self) -> float: + """ + Returns a float number in [0, 1] which indicates the current progress. + """ if len(self._items) == 0: return 0 else: return self._current_page_index / len(self._items) - # Indicates if the current page is the last page. @pyqtProperty(bool, notify = currentPageIndexChanged) def isCurrentPageLast(self) -> bool: + """ + Indicates if the current page is the last page. + """ return self._current_page_index == len(self._items) - 1 def _setCurrentPageIndex(self, page_index: int) -> None: @@ -91,17 +96,22 @@ class WelcomePagesModel(ListModel): self._current_page_index = page_index self.currentPageIndexChanged.emit() - # Ends the Welcome-Pages. Put as a separate function for cases like the 'decline' in the User-Agreement. @pyqtSlot() def atEnd(self) -> None: + """ + Ends the Welcome-Pages. Put as a separate function for cases like the 'decline' in the User-Agreement. + """ self.allFinished.emit() self.resetState() - # Goes to the next page. - # If "from_index" is given, it will look for the next page to show starting from the "from_index" page instead of - # the "self._current_page_index". @pyqtSlot() def goToNextPage(self, from_index: Optional[int] = None) -> None: + """ + Goes to the next page. + If "from_index" is given, it will look for the next page to show starting from the "from_index" page instead of + the "self._current_page_index". + """ + # Look for the next page that should be shown current_index = self._current_page_index if from_index is None else from_index while True: @@ -137,9 +147,11 @@ class WelcomePagesModel(ListModel): # Move to the next page self._setCurrentPageIndex(next_page_index) - # Goes to the previous page. If there's no previous page, do nothing. @pyqtSlot() def goToPreviousPage(self) -> None: + """ + Goes to the previous page. If there's no previous page, do nothing. + """ if len(self._previous_page_indices_stack) == 0: Logger.log("i", "No previous page, do nothing") return @@ -148,9 +160,9 @@ class WelcomePagesModel(ListModel): self._current_page_index = previous_page_index self.currentPageIndexChanged.emit() - # Sets the current page to the given page ID. If the page ID is not found, do nothing. @pyqtSlot(str) def goToPage(self, page_id: str) -> None: + """Sets the current page to the given page ID. If the page ID is not found, do nothing.""" page_index = self.getPageIndexById(page_id) if page_index is None: # FIXME: If we cannot find the next page, we cannot do anything here. @@ -165,18 +177,22 @@ class WelcomePagesModel(ListModel): # Find the next page to show starting from the "page_index" self.goToNextPage(from_index = page_index) - # Checks if the page with the given index should be shown by calling the "should_show_function" associated with it. - # If the function is not present, returns True (show page by default). def _shouldPageBeShown(self, page_index: int) -> bool: + """ + Checks if the page with the given index should be shown by calling the "should_show_function" associated with + it. If the function is not present, returns True (show page by default). + """ next_page_item = self.getItem(page_index) should_show_function = next_page_item.get("should_show_function", lambda: True) return should_show_function() - # Resets the state of the WelcomePagesModel. This functions does the following: - # - Resets current_page_index to 0 - # - Clears the previous page indices stack @pyqtSlot() def resetState(self) -> None: + """ + Resets the state of the WelcomePagesModel. This functions does the following: + - Resets current_page_index to 0 + - Clears the previous page indices stack + """ self._current_page_index = 0 self._previous_page_indices_stack.clear() @@ -188,8 +204,8 @@ class WelcomePagesModel(ListModel): def shouldShowWelcomeFlow(self) -> bool: return self._should_show_welcome_flow - # Gets the page index with the given page ID. If the page ID doesn't exist, returns None. def getPageIndexById(self, page_id: str) -> Optional[int]: + """Gets the page index with the given page ID. If the page ID doesn't exist, returns None.""" page_idx = None for idx, page_item in enumerate(self._items): if page_item["id"] == page_id: @@ -197,8 +213,8 @@ class WelcomePagesModel(ListModel): break return page_idx - # Convenience function to get QUrl path to pages that's located in "resources/qml/WelcomePages". def _getBuiltinWelcomePagePath(self, page_filename: str) -> "QUrl": + """Convenience function to get QUrl path to pages that's located in "resources/qml/WelcomePages".""" from cura.CuraApplication import CuraApplication return QUrl.fromLocalFile(Resources.getPath(CuraApplication.ResourceTypes.QmlFiles, os.path.join("WelcomePages", page_filename))) @@ -223,10 +239,11 @@ class WelcomePagesModel(ListModel): show_whatsnew_only = has_active_machine and has_app_just_upgraded # FIXME: This is a hack. Because of the circular dependency between MachineManager, ExtruderManager, and - # possibly some others, setting the initial active machine is not done when the MachineManager gets initialized. - # So at this point, we don't know if there will be an active machine or not. It could be that the active machine - # files are corrupted so we cannot rely on Preferences either. This makes sure that once the active machine - # gets changed, this model updates the flags, so it can decide whether to show the welcome flow or not. + # possibly some others, setting the initial active machine is not done when the MachineManager gets + # initialized. So at this point, we don't know if there will be an active machine or not. It could be that + # the active machine files are corrupted so we cannot rely on Preferences either. This makes sure that once + # the active machine gets changed, this model updates the flags, so it can decide whether to show the + # welcome flow or not. should_show_welcome_flow = show_complete_flow or show_whatsnew_only if should_show_welcome_flow != self._should_show_welcome_flow: self._should_show_welcome_flow = should_show_welcome_flow @@ -280,17 +297,19 @@ class WelcomePagesModel(ListModel): self._pages = pages_to_show self.setItems(self._pages) - # For convenience, inject the default "next" button text to each item if it's not present. def setItems(self, items: List[Dict[str, Any]]) -> None: + # For convenience, inject the default "next" button text to each item if it's not present. for item in items: if "next_page_button_text" not in item: item["next_page_button_text"] = self._default_next_button_text super().setItems(items) - # Indicates if the machine action panel should be shown by checking if there's any first start machine actions - # available. def shouldShowMachineActions(self) -> bool: + """ + Indicates if the machine action panel should be shown by checking if there's any first start machine actions + available. + """ global_stack = self._application.getMachineManager().activeMachine if global_stack is None: return False @@ -312,6 +331,3 @@ class WelcomePagesModel(ListModel): def addPage(self) -> None: pass - - -__all__ = ["WelcomePagesModel"] From 22fcdf5c7b179fe94c1b2199f8c248f4dcba1ad0 Mon Sep 17 00:00:00 2001 From: Jaime van Kessel Date: Wed, 1 Dec 2021 10:30:52 +0100 Subject: [PATCH 4/5] Change whatsnew to whats_new They are seperate words, so should have an _ in between CURA-8696 --- cura/UI/WelcomePagesModel.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cura/UI/WelcomePagesModel.py b/cura/UI/WelcomePagesModel.py index ec0958d07e..41e45bacd9 100644 --- a/cura/UI/WelcomePagesModel.py +++ b/cura/UI/WelcomePagesModel.py @@ -229,14 +229,14 @@ class WelcomePagesModel(ListModel): self._initialize() def _initialize(self, update_should_show_flag: bool = True) -> None: - show_whatsnew_only = False + show_whats_new_only = False if update_should_show_flag: has_active_machine = self._application.getMachineManager().activeMachine is not None has_app_just_upgraded = self._application.hasJustUpdatedFromOldVersion() # Only show the what's new dialog if there's no machine and we have just upgraded show_complete_flow = not has_active_machine - show_whatsnew_only = has_active_machine and has_app_just_upgraded + show_whats_new_only = has_active_machine and has_app_just_upgraded # FIXME: This is a hack. Because of the circular dependency between MachineManager, ExtruderManager, and # possibly some others, setting the initial active machine is not done when the MachineManager gets @@ -244,7 +244,7 @@ class WelcomePagesModel(ListModel): # the active machine files are corrupted so we cannot rely on Preferences either. This makes sure that once # the active machine gets changed, this model updates the flags, so it can decide whether to show the # welcome flow or not. - should_show_welcome_flow = show_complete_flow or show_whatsnew_only + should_show_welcome_flow = show_complete_flow or show_whats_new_only if should_show_welcome_flow != self._should_show_welcome_flow: self._should_show_welcome_flow = should_show_welcome_flow self.shouldShowWelcomeFlowChanged.emit() @@ -291,7 +291,7 @@ class WelcomePagesModel(ListModel): ] pages_to_show = all_pages_list - if show_whatsnew_only: + if show_whats_new_only: pages_to_show = list(filter(lambda x: x["id"] == "whats_new", all_pages_list)) self._pages = pages_to_show From 83b56726ba8cbc606d08ed4457ab9682e3fa84a1 Mon Sep 17 00:00:00 2001 From: Jaime van Kessel Date: Wed, 1 Dec 2021 10:33:05 +0100 Subject: [PATCH 5/5] CHange _getBuiltinWelcomePagePath to be static SHould have been static, so boyscouting it now CURA-8696 --- cura/UI/WelcomePagesModel.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cura/UI/WelcomePagesModel.py b/cura/UI/WelcomePagesModel.py index 41e45bacd9..890e34a31e 100644 --- a/cura/UI/WelcomePagesModel.py +++ b/cura/UI/WelcomePagesModel.py @@ -213,7 +213,8 @@ class WelcomePagesModel(ListModel): break return page_idx - def _getBuiltinWelcomePagePath(self, page_filename: str) -> "QUrl": + @staticmethod + def _getBuiltinWelcomePagePath(page_filename: str) -> QUrl: """Convenience function to get QUrl path to pages that's located in "resources/qml/WelcomePages".""" from cura.CuraApplication import CuraApplication return QUrl.fromLocalFile(Resources.getPath(CuraApplication.ResourceTypes.QmlFiles,