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

dbeaver/dbeaver#20602 load controls for extra pages #22616

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ private IDataSourceConnectionEditor getOriginalConnectionEditor() {
public void activatePage() {
if (connectionEditor == null) {
createProviderPage(getControl().getParent());
//UIUtils.resizeShell(getWizard().getContainer().getShell());
}

Control control = getControl();
Expand Down Expand Up @@ -192,6 +191,31 @@ public void activatePage() {
//getContainer().updateTitleBar();
}

private void createAdditionalRequiredPages() {
Copy link
Member

Choose a reason for hiding this comment

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

I doubt about calling this method create... I would rather expact to call it like load.. or smth like that. Yes, you call createControl method, but the control is not visible and so on..

if (connectionEditor instanceof IDialogPageProvider dialogPageProvider) {
IDialogPage[] requiredDialogPages = dialogPageProvider.getRequiredDialogPages();
// potentially n^2 but we don't have a lot of required pages...
if (requiredDialogPages != null) {
for (IDialogPage requiredDialogPage : requiredDialogPages) {
for (CTabItem item : tabFolder.getItems()) {
if (item.getData().equals(requiredDialogPage)) {
Composite panel = (Composite) item.getControl();
try {
panel.setRedraw(false);
requiredDialogPage.createControl(panel);
Dialog.applyDialogFont(panel);
panel.layout(true, true);
requiredDialogPage.setVisible(false);
} finally {
panel.setRedraw(true);
E1izabeth marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}
}
}
}

@Override
public void deactivatePage() {
DataSourceDescriptor connectionInfo = getActiveDataSource();
Expand Down Expand Up @@ -334,6 +358,7 @@ public void widgetSelected(SelectionEvent e) {
setErrorMessage("Can't create settings dialog: " + ex.getMessage());
}
parent.layout();
createAdditionalRequiredPages();
}

@NotNull
Expand Down Expand Up @@ -367,8 +392,7 @@ private ToolBar createChevron(@NotNull List<IDialogPage> pages) {
}

private boolean confirmTabClose(@NotNull CTabItem item) {
if (item.getData() instanceof ConnectionPageNetworkHandler) {
final ConnectionPageNetworkHandler page = (ConnectionPageNetworkHandler) item.getData();
if (item.getData() instanceof ConnectionPageNetworkHandler page) {
final NetworkHandlerDescriptor descriptor = page.getHandlerDescriptor();

final int decision = ConfirmationDialog.confirmAction(
Expand Down Expand Up @@ -504,7 +528,7 @@ public void updateButtons() {

@Override
public boolean openDriverEditor() {
DriverEditDialog dialog = new DriverEditDialog(wizard.getShell(), (DriverDescriptor) this.getDriver());
DriverEditDialog dialog = new DriverEditDialog(wizard.getShell(), this.getDriver());
return dialog.open() == IDialogConstants.OK_ID;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.eclipse.swt.layout.GridLayout;
import org.eclipse.swt.widgets.*;
import org.jkiss.code.NotNull;
import org.jkiss.code.Nullable;
import org.jkiss.dbeaver.ext.oracle.model.OracleConstants;
import org.jkiss.dbeaver.ext.oracle.model.auth.OracleAuthModelDatabaseNative;
import org.jkiss.dbeaver.ext.oracle.model.auth.OracleAuthOS;
Expand Down Expand Up @@ -72,7 +73,8 @@ public class OracleConnectionPage extends ConnectionPageWithAuth implements IDia
private TextWithOpenFolder tnsPathText;

private boolean activated = false;
private Image logoImage;
private final Image logoImage;
private OracleConnectionExtraPage oracleConnectionExtraPage;

public OracleConnectionPage() {
logoImage = createImage("icons/oracle_logo.png"); //$NON-NLS-1
Expand Down Expand Up @@ -450,12 +452,17 @@ public void widgetDefaultSelected(SelectionEvent e) {
}

@Override
public IDialogPage[] getDialogPages(boolean extrasOnly, boolean forceCreate)
{
return new IDialogPage[] {
new OracleConnectionExtraPage(),
public IDialogPage[] getDialogPages(boolean extrasOnly, boolean forceCreate) {
oracleConnectionExtraPage = new OracleConnectionExtraPage();
return new IDialogPage[]{
oracleConnectionExtraPage,
new DriverPropertiesDialogPage(this),
};
}

@Nullable
@Override
public IDialogPage[] getRequiredDialogPages() {
return new IDialogPage[]{oracleConnectionExtraPage};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.eclipse.swt.layout.GridData;
import org.eclipse.swt.layout.GridLayout;
import org.eclipse.swt.widgets.*;
import org.jkiss.code.Nullable;
import org.jkiss.dbeaver.ext.postgresql.PostgreConstants;
import org.jkiss.dbeaver.ext.postgresql.PostgreMessages;
import org.jkiss.dbeaver.ext.postgresql.PostgreUtils;
Expand All @@ -51,7 +52,7 @@
* PostgreConnectionPage
*/
public class PostgreConnectionPage extends ConnectionPageWithAuth implements IDialogPageProvider {

private Text urlText;
private Text hostText;
private Text portText;
Expand All @@ -60,6 +61,7 @@ public class PostgreConnectionPage extends ConnectionPageWithAuth implements IDi
private Text roleText; //TODO: make it a combo and fill it with appropriate roles
private ClientHomesSelector homesSelector;
private boolean activated = false;
private PostgreConnectionPageAdvanced postgreConnectionPageAdvanced;

@Override
public void dispose() {
Expand Down Expand Up @@ -317,12 +319,19 @@ public void saveSettings(DBPDataSourceContainer dataSource) {

@Override
public IDialogPage[] getDialogPages(boolean extrasOnly, boolean forceCreate) {
return new IDialogPage[] {
new PostgreConnectionPageAdvanced(),
postgreConnectionPageAdvanced = new PostgreConnectionPageAdvanced();
return new IDialogPage[]{
postgreConnectionPageAdvanced,
new DriverPropertiesDialogPage(this)
};
}


@Nullable
@Override
public IDialogPage[] getRequiredDialogPages() {
return new IDialogPage[]{postgreConnectionPageAdvanced};
}

private void updateUrl() {
DBPDataSourceContainer dataSourceContainer = site.getActiveDataSource();
saveSettings(dataSourceContainer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,17 @@ public interface IDialogPageProvider {
@Nullable
IDialogPage[] getDialogPages(boolean extrasOnly, boolean forceCreate);

/**
* Pages what should be saved during creation, even if they were
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Pages what should be saved during creation, even if they were
* Pages that should be saved during creation, even if they were

* never opened
*
* @return array of IDialogPage[] to create at the initialisation,
* null if no additional pages should be created.
*/
@Nullable
default IDialogPage[] getRequiredDialogPages() {
Copy link
Member

Choose a reason for hiding this comment

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

Method name is not descriptive enough. I couldn't understand what does it mean, until I read the comment. I'd propose something like getPagesContainingSettings. Maybe you can figure out better name, but don't left just as is, please. It's not clear what does Required means - required for what?

return null;
}


}