Skip to content

Cb 4629 end user preferences#2546

Merged
alexander-skoblikov merged 20 commits intodevelfrom
CB-4629-end-user-preferences
Apr 16, 2024
Merged

Cb 4629 end user preferences#2546
alexander-skoblikov merged 20 commits intodevelfrom
CB-4629-end-user-preferences

Conversation

@alexander-skoblikov
Copy link
Copy Markdown
Collaborator

No description provided.

@Wroud Wroud requested a review from sergeyteleshev April 12, 2024 07:39
Comment thread server/bundles/io.cloudbeaver.model/src/io/cloudbeaver/WebProjectImpl.java Outdated
<ToolsAction icon="admin-cancel" viewBox="0 0 24 24" disabled={!changed} onClick={handleReset}>
{translate('ui_processing_cancel')}
</ToolsAction>
<ToolsAction icon="admin-cancel" viewBox="0 0 24 24" onClick={handleRestoreDefaults}>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I dont like we have 2 similar icons. This is misleading the user

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

fixed

}

function handleRestoreDefaults() {
userSettingsService.restoreDefaults();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Usually restore settings goes this scenario:

  1. press restore settings
  2. popup "are you sure you want to restore settings"
  3. click yes/no
  4. settings applies

Since we have another pipline here, which is fine, by the way. I believe we should add here some kind of notification here that we pasted here default settings but did not save it yet.

Cuase for example: user had first 5 setttings as default. All settings below visible window area were different from default. You clicked this button and user did not see that something were changed. We need to cover this case so user would not have been missleaded. Generally we need to notify somehow that we pasted something successfully

Or we can go always go first scenario

getHandler(contexts: IDataContextProvider): IMenuHandler | null {
const menu = contexts.getOwn(DATA_CONTEXT_MENU);

handlers: for (const handler of this.handlers.values()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

seems like this logic duplicates in handlig actions and bindings. I beleive we can unify it in function with this cycle. but may be it is not a bad idea to leave it as it is for flexibility reasons

@sergeyteleshev sergeyteleshev self-requested a review April 12, 2024 13:30
@Wroud Wroud force-pushed the CB-4629-end-user-preferences branch from ea9e39c to d28715c Compare April 16, 2024 14:53
@alexander-skoblikov alexander-skoblikov merged commit 3e71a7a into devel Apr 16, 2024
@alexander-skoblikov alexander-skoblikov deleted the CB-4629-end-user-preferences branch April 18, 2024 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants