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

Improve Synchronization UI #897

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
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
4 changes: 3 additions & 1 deletion GTG/backends/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
from GTG.backends.generic_backend import GenericBackend

log = logging.getLogger(__name__)

inactive_modules = set()
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be better to move this to the BackendFactory class below. The Borg class it inherits is basically making it a singleton (state shared on every instance).


class BackendFactory(Borg):
"""
Expand Down Expand Up @@ -67,10 +67,12 @@ def __init__(self):
# Something is wrong with this backend, skipping
log.warning("Backend %s could not be loaded: %r",
module_name, exception)
inactive_modules.append(module_name)
Copy link
Contributor

@Neui Neui May 29, 2022

Choose a reason for hiding this comment

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

Suggested change
inactive_modules.append(module_name)
inactive_modules.add(module_name)
Traceback (most recent call last):
  File "/app/lib/python3.9/site-packages/GTG/backends/__init__.py", line 65, in __init__
    __import__(extended_module_name)
  File "/app/lib/python3.9/site-packages/GTG/backends/backend_caldav.py", line 30, in <module>
    import caldav
ModuleNotFoundError: No module named 'caldav'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/app/lib/python3.9/site-packages/GTG/gtk/application.py", line 124, in do_startup
    for backend_dic in BackendFactory().get_saved_backends_list():
  File "/app/lib/python3.9/site-packages/GTG/backends/__init__.py", line 70, in __init__
    inactive_modules.append(module_name)
AttributeError: 'set' object has no attribute 'append'

You need to use .add() since it is a set, not an list.

continue
except Exception:
# Other exception log as errors
log.exception("Malformated backend %s:", module_name)
inactive_modules.append(module_name)
Copy link
Contributor

@Neui Neui May 29, 2022

Choose a reason for hiding this comment

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

Suggested change
inactive_modules.append(module_name)
inactive_modules.add(module_name)

Ditto. You need to use .add() since it is a set, not an list.

continue

def browse_subclasses(cls):
Expand Down
11 changes: 11 additions & 0 deletions GTG/gtk/backends/addpanel.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
from GTG.backends import BackendFactory
from gettext import gettext as _, ngettext

from GTG.backends import inactive_modules


class AddPanel(Gtk.Box):
"""
Expand Down Expand Up @@ -170,7 +172,16 @@ def on_combo_changed(self, widget=None):
"""
backend_name = self.combo_types.get_selected()
if backend_name is None:
if 'backend_caldav' in inactive_modules:
markup = '<big>Error: Python package \'caldev\' not installed.</big>'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
markup = '<big>Error: Python package \'caldev\' not installed.</big>'
markup = '<big>{}</big>'.format(_('Error: Python package \'caldav\' not installed.'))

Typo in caldav, and make it translatable.

self.label_name.set_markup(markup)
return


markup = '<big>Error: An unknown backend could not be loaded.</big>'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
markup = '<big>Error: An unknown backend could not be loaded.</big>'
markup = '<big>{}</big>'.format(_('Error: An unknown backend could not be loaded.'))

Make it translatable.

self.label_name.set_markup(markup)
return

backend = BackendFactory().get_backend(backend_name)
self.label_description.set_markup(backend.Backend.get_description())

Expand Down
8 changes: 5 additions & 3 deletions GTG/gtk/backends/configurepanel.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
# this program. If not, see <http://www.gnu.org/licenses/>.
# -----------------------------------------------------------------------------

from gi.repository import Gtk
from gi.repository import Gtk, GLib

from gettext import gettext as _
from GTG.gtk.backends.parameters_ui import ParametersUI
Expand Down Expand Up @@ -139,7 +139,7 @@ def refresh_sync_button(self):
"""
Refreshes the state of the button that enables the backend
"""
self.sync_button.set_sensitive(not self.backend.is_default())
self.sync_button.set_visible(not self.backend.is_default())
if self.backend.is_enabled():
label = _("Disable syncing")
else:
Expand All @@ -151,7 +151,9 @@ def refresh_sync_status_label(self):
Refreshes the Gtk.Label that shows the current state of this backend
"""
if self.backend.is_default():
label = _("This is the default synchronization service")
xml_folder = GLib.filename_to_uri("/home/" + GLib.get_user_name() + "/.var/app/org.gnome.GTG/data/gtg/gtg_data.xml")
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't hardcode paths like that, especially this is flatpak only and not everyone uses the flatpak version (such as distros, developers or users who test locally/want the latest version).

Seems self.backend.get_path() should work.

def get_path(self) -> str:


label = "This is the default file storage backend. <a href='{}'>{}</a>.".format(xml_folder, _("Open the XML data file"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
label = "This is the default file storage backend. <a href='{}'>{}</a>.".format(xml_folder, _("Open the XML data file"))
label = _("This is the default file storage backend. <a href='{}'>Open the XML data file</a>.").format(xml_folder)

The outer text wasn't marked as translatable. Also you can inline the text for more context to the translators.

else:
if self.backend.is_enabled():
label = _("Syncing is enabled.")
Expand Down