-
Notifications
You must be signed in to change notification settings - Fork 14
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
Project history : Add a version viewer windows in the plugin #630
Conversation
…roject-history-refactor
self.current_page = 1 | ||
self.per_page = 50 | ||
|
||
version_count = self.mc.project_versions_count(self.project_path) |
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.
Shouldn't the mc
network calls be done on the separate thread?
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.
Yes but we need these info right away, we agreed with @tomasMizera to let it as is for now as it would require more refactor to make this async
|
||
self.setWindowTitle(f"Changes Viewer | {version_name}") | ||
|
||
self.version_details = self.mc.project_version_info(self.mp.project_id(), version_name) |
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.
This is also done on the main thread slowing down the ui response when the user clicks.
Could this be run in a separate thread?
Could we at least cache these responses? (are they needed when there is already a .cache
folder?)
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.
Same as above
Mergin/version_viewer_dialog.py
Outdated
d = min(extent.width(), extent.height()) | ||
if d == 0: | ||
d = 1 | ||
extent = extent.buffered(d * 0.07) | ||
|
||
extent = self.map_canvas.mapSettings().layerExtentToOutputExtent(layers[0], extent) | ||
|
||
self.map_canvas.setExtent(extent) |
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.
Not sure what you're trying to do here...
Is it:
d = min(extent.width(), extent.height()) | |
if d == 0: | |
d = 1 | |
extent = extent.buffered(d * 0.07) | |
extent = self.map_canvas.mapSettings().layerExtentToOutputExtent(layers[0], extent) | |
self.map_canvas.setExtent(extent) | |
extent = self.map_canvas.mapSettings().layerExtentToOutputExtent(layers[0], extent) | |
extent.scale(1.07) | |
self.map_canvas.setExtent(extent) |
?
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.
I think the intend is the same is both code, I'm not sure why I some point we defaulted to 1 ?
* Streamline function name
Let's merge this now and continue fixing from |
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.
This PR is a refactor and enhancement to bring a new history window to browse the history of a project directly from the plugin
To open the new windows a new toolbar button has been added.
The windows is divided in three parts for the users to be able to:
This PR is marked as draft because some testing are still required and the feature still have rough edges
For reviewer a brief technical documentation is available in the comment