NSLS2 / pymca

PyMca Toolkit git repository
Other
0 stars 3 forks source link

Data view for Tiled Selection Dialog #17

Closed AbbyGi closed 2 weeks ago

padraic-shafer commented 2 weeks ago

@AbbyGi Following my latest review, I realized that set_root_client() is really doing 2 things. Breaking them up into single-responsibility functions might look something like this...

    def on_connect_clicked(self, checked: bool = False):
        """Handle a button click to connect to the Tiled client."""
        _logger.debug("TiledCatalogSelector.on_connect_clicked()...")

        if self.client:
            # TODO: Clean-up previously connected client?
            ...

        self.connect_client()
        self.reset_client_view()

    def connect_client(self) -> None:
        """Connect the model's Tiled client to the Tiled server at URL.

            Emits the 'client_connection_error' signal when client does not connect.
        """
        try:
            new_client = self.client_from_url(self.url)
        except Exception as exception:
            error_message = str(exception)
            _logger.error(error_message)
            self.client_connection_error.emit(error_message)
            return

        self._client = new_client
        self.client_connected.emit(
            self._client.uri,
            str(self._client.context.api_uri),
        )

    def reset_client_view(self) -> None:
        """Prepare the model to receive content from a Tiled server.

            Emits the 'table_changed' signal when a client is defined.
        """
        self.node_path_parts = ()
        self._current_page = 0
        if self.client is not None:
            self.table_changed.emit(self.node_path_parts)
padraic-shafer commented 2 weeks ago

What do you think about splitting up set_root_client() into connect_client () + reset_client_view()?...or something along those lines? https://github.com/NSLS2/pymca/pull/17#issuecomment-2431943794

AbbyGi commented 2 weeks ago

I addressed splitting up set_root_client in 12aca0c50fae951101cc76361069bbd8aadd666d.