Обсуждение: [pgAdmin4][RM#3155] Allow user to lock the Layout

Поиск
Список
Период
Сортировка

[pgAdmin4][RM#3155] Allow user to lock the Layout

От
Murtuza Zabuawala
Дата:
Hi,

PFA patch which will allow user to lock the panels and it will not allow user to drag & drop them.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Вложения

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Dave Page
Дата:
Hi

On Thu, Mar 29, 2018 at 2:15 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA patch which will allow user to lock the panels and it will not allow user to drag & drop them.

Tests pass, but when I lock the layout, I can still drag panels and adjust the splitters etc. After doing so,  reset the layout and now have the broken layout seen in the attached screenshot. I have rebuilt the bundle, reloaded etc. 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Joao De Almeida Pereira
Дата:
Hi Murtuza,

After changing the setting in the preferences nothing happened, we had to reset the layout or refresh the app to see it working. It only looks the right side. Was this the intended behavior?

Not sure if this is the expected behavior or not. I would expect that any change I do in the preferences would start working after I press the Save button. This also happens with other preferences that only take effect after refresh on the browser.
This being said, not sure if having the templated variable in the javascript file is the best approach in this case.

Do you think you can remove the requirejs tags on the tests?

At the testing file you do not need to create 3 different variables for the panels, you can reuse it, because the beforeEach will run for every test

Thanks
Joao

On Thu, Mar 29, 2018 at 9:48 AM Dave Page <dpage@pgadmin.org> wrote:
Hi

On Thu, Mar 29, 2018 at 2:15 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA patch which will allow user to lock the panels and it will not allow user to drag & drop them.

Tests pass, but when I lock the layout, I can still drag panels and adjust the splitters etc. After doing so,  reset the layout and now have the broken layout seen in the attached screenshot. I have rebuilt the bundle, reloaded etc. 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Murtuza Zabuawala
Дата:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.

@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

By visiting wcDocker github page It looks like it not actively maintained.
What do you suggest how should we tackle this issue?

For time being, I've created subtask for this issue https://redmine.postgresql.org/issues/3243

Thanks,
Murtuza
 ​
On Thu, Mar 29, 2018 at 8:57 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

After changing the setting in the preferences nothing happened, we had to reset the layout or refresh the app to see it working. It only looks the right side. Was this the intended behavior?

Not sure if this is the expected behavior or not. I would expect that any change I do in the preferences would start working after I press the Save button. This also happens with other preferences that only take effect after refresh on the browser.
This being said, not sure if having the templated variable in the javascript file is the best approach in this case.

Do you think you can remove the requirejs tags on the tests?

At the testing file you do not need to create 3 different variables for the panels, you can reuse it, because the beforeEach will run for every test

Thanks
Joao

On Thu, Mar 29, 2018 at 9:48 AM Dave Page <dpage@pgadmin.org> wrote:
Hi

On Thu, Mar 29, 2018 at 2:15 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA patch which will allow user to lock the panels and it will not allow user to drag & drop them.

Tests pass, but when I lock the layout, I can still drag panels and adjust the splitters etc. After doing so,  reset the layout and now have the broken layout seen in the attached screenshot. I have rebuilt the bundle, reloaded etc. 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Вложения

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Joao De Almeida Pereira
Дата:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

By visiting wcDocker github page It looks like it not actively maintained.
What do you suggest how should we tackle this issue?


I think this should be moved to a different thread, because at this point in time we have 3 of our core libraries that are no longer maintained/supported/under active development that I know out of my head. (ACITree, Backbone and wcDocker). I might even add to the mix jquery 1.11.2 because it stopped being actively developed and supported after May 20 of 2016.
 
For time being, I've created subtask for this issue https://redmine.postgresql.org/issues/3243

Thanks,
Murtuza
 ​
On Thu, Mar 29, 2018 at 8:57 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

After changing the setting in the preferences nothing happened, we had to reset the layout or refresh the app to see it working. It only looks the right side. Was this the intended behavior?

Not sure if this is the expected behavior or not. I would expect that any change I do in the preferences would start working after I press the Save button. This also happens with other preferences that only take effect after refresh on the browser.
This being said, not sure if having the templated variable in the javascript file is the best approach in this case.

Do you think you can remove the requirejs tags on the tests?

At the testing file you do not need to create 3 different variables for the panels, you can reuse it, because the beforeEach will run for every test

Thanks
Joao

On Thu, Mar 29, 2018 at 9:48 AM Dave Page <dpage@pgadmin.org> wrote:
Hi

On Thu, Mar 29, 2018 at 2:15 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA patch which will allow user to lock the panels and it will not allow user to drag & drop them.

Tests pass, but when I lock the layout, I can still drag panels and adjust the splitters etc. After doing so,  reset the layout and now have the broken layout seen in the attached screenshot. I have rebuilt the bundle, reloaded etc. 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Murtuza Zabuawala
Дата:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

By visiting wcDocker github page It looks like it not actively maintained.
What do you suggest how should we tackle this issue?


I think this should be moved to a different thread, because at this point in time we have 3 of our core libraries that are no longer maintained/supported/under active development that I know out of my head. (ACITree, Backbone and wcDocker). I might even add to the mix jquery 1.11.2 because it stopped being actively developed and supported after May 20 of 2016.
​Sure, I'll send separate email.​
 
 
For time being, I've created subtask for this issue https://redmine.postgresql.org/issues/3243

Thanks,
Murtuza
 ​
On Thu, Mar 29, 2018 at 8:57 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

After changing the setting in the preferences nothing happened, we had to reset the layout or refresh the app to see it working. It only looks the right side. Was this the intended behavior?

Not sure if this is the expected behavior or not. I would expect that any change I do in the preferences would start working after I press the Save button. This also happens with other preferences that only take effect after refresh on the browser.
This being said, not sure if having the templated variable in the javascript file is the best approach in this case.

Do you think you can remove the requirejs tags on the tests?

At the testing file you do not need to create 3 different variables for the panels, you can reuse it, because the beforeEach will run for every test

Thanks
Joao

On Thu, Mar 29, 2018 at 9:48 AM Dave Page <dpage@pgadmin.org> wrote:
Hi

On Thu, Mar 29, 2018 at 2:15 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA patch which will allow user to lock the panels and it will not allow user to drag & drop them.

Tests pass, but when I lock the layout, I can still drag panels and adjust the splitters etc. After doing so,  reset the layout and now have the broken layout seen in the attached screenshot. I have rebuilt the bundle, reloaded etc. 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Вложения

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Joao De Almeida Pereira
Дата:
Hi Murtuza,

Everything seems to work, the tests are all green and the linter is fixed.

As we stated in an previous email, the direction we would love the application to go is a more robust Javascript Front-End that would rely in the Backend to provide data. Adding more things to templated Javascript files feels like a step back and something that we will in the future have to convert into AJAX calls and JSON responses.
As discussed before our idea is to remove all the javascript templated files.

How hard do you think it would be to do this implementation without using templates?

Thanks
Victoria & Joao

On Tue, Apr 3, 2018 at 7:57 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

By visiting wcDocker github page It looks like it not actively maintained.
What do you suggest how should we tackle this issue?


I think this should be moved to a different thread, because at this point in time we have 3 of our core libraries that are no longer maintained/supported/under active development that I know out of my head. (ACITree, Backbone and wcDocker). I might even add to the mix jquery 1.11.2 because it stopped being actively developed and supported after May 20 of 2016.
​Sure, I'll send separate email.​
 
 
For time being, I've created subtask for this issue https://redmine.postgresql.org/issues/3243

Thanks,
Murtuza
 ​
On Thu, Mar 29, 2018 at 8:57 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

After changing the setting in the preferences nothing happened, we had to reset the layout or refresh the app to see it working. It only looks the right side. Was this the intended behavior?

Not sure if this is the expected behavior or not. I would expect that any change I do in the preferences would start working after I press the Save button. This also happens with other preferences that only take effect after refresh on the browser.
This being said, not sure if having the templated variable in the javascript file is the best approach in this case.

Do you think you can remove the requirejs tags on the tests?

At the testing file you do not need to create 3 different variables for the panels, you can reuse it, because the beforeEach will run for every test

Thanks
Joao

On Thu, Mar 29, 2018 at 9:48 AM Dave Page <dpage@pgadmin.org> wrote:
Hi

On Thu, Mar 29, 2018 at 2:15 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA patch which will allow user to lock the panels and it will not allow user to drag & drop them.

Tests pass, but when I lock the layout, I can still drag panels and adjust the splitters etc. After doing so,  reset the layout and now have the broken layout seen in the attached screenshot. I have rebuilt the bundle, reloaded etc. 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Dave Page
Дата:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 

By visiting wcDocker github page It looks like it not actively maintained.
What do you suggest how should we tackle this issue?

It may not have been updated recently, but the lead developer answered your questions pretty quickly. Maybe he'll be open to a pull request if we can figure out how to lock the layout of the frame as well.
 


I think this should be moved to a different thread, because at this point in time we have 3 of our core libraries that are no longer maintained/supported/under active development that I know out of my head. (ACITree, Backbone and wcDocker). I might even add to the mix jquery 1.11.2 because it stopped being actively developed and supported after May 20 of 2016.
​Sure, I'll send separate email.​
 
 
For time being, I've created subtask for this issue https://redmine.postgresql.org/issues/3243

Thanks,
Murtuza
 ​
On Thu, Mar 29, 2018 at 8:57 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

After changing the setting in the preferences nothing happened, we had to reset the layout or refresh the app to see it working. It only looks the right side. Was this the intended behavior?

Not sure if this is the expected behavior or not. I would expect that any change I do in the preferences would start working after I press the Save button. This also happens with other preferences that only take effect after refresh on the browser.
This being said, not sure if having the templated variable in the javascript file is the best approach in this case.

Do you think you can remove the requirejs tags on the tests?

At the testing file you do not need to create 3 different variables for the panels, you can reuse it, because the beforeEach will run for every test

Thanks
Joao

On Thu, Mar 29, 2018 at 9:48 AM Dave Page <dpage@pgadmin.org> wrote:
Hi

On Thu, Mar 29, 2018 at 2:15 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA patch which will allow user to lock the panels and it will not allow user to drag & drop them.

Tests pass, but when I lock the layout, I can still drag panels and adjust the splitters etc. After doing so,  reset the layout and now have the broken layout seen in the attached screenshot. I have rebuilt the bundle, reloaded etc. 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company





--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Murtuza Zabuawala
Дата:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


 

By visiting wcDocker github page It looks like it not actively maintained.
What do you suggest how should we tackle this issue?

It may not have been updated recently, but the lead developer answered your questions pretty quickly. Maybe he'll be open to a pull request if we can figure out how to lock the layout of the frame as well.
 


I think this should be moved to a different thread, because at this point in time we have 3 of our core libraries that are no longer maintained/supported/under active development that I know out of my head. (ACITree, Backbone and wcDocker). I might even add to the mix jquery 1.11.2 because it stopped being actively developed and supported after May 20 of 2016.
​Sure, I'll send separate email.​
 
 
For time being, I've created subtask for this issue https://redmine.postgresql.org/issues/3243

Thanks,
Murtuza
 ​
On Thu, Mar 29, 2018 at 8:57 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

After changing the setting in the preferences nothing happened, we had to reset the layout or refresh the app to see it working. It only looks the right side. Was this the intended behavior?

Not sure if this is the expected behavior or not. I would expect that any change I do in the preferences would start working after I press the Save button. This also happens with other preferences that only take effect after refresh on the browser.
This being said, not sure if having the templated variable in the javascript file is the best approach in this case.

Do you think you can remove the requirejs tags on the tests?

At the testing file you do not need to create 3 different variables for the panels, you can reuse it, because the beforeEach will run for every test

Thanks
Joao

On Thu, Mar 29, 2018 at 9:48 AM Dave Page <dpage@pgadmin.org> wrote:
Hi

On Thu, Mar 29, 2018 at 2:15 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA patch which will allow user to lock the panels and it will not allow user to drag & drop them.

Tests pass, but when I lock the layout, I can still drag panels and adjust the splitters etc. After doing so,  reset the layout and now have the broken layout seen in the attached screenshot. I have rebuilt the bundle, reloaded etc. 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company





--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Вложения

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Dave Page
Дата:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?

There must be some way we can lock a tab that's not part of a group.
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Murtuza Zabuawala
Дата:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Dave Page
Дата:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Murtuza Zabuawala
Дата:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Dave Page
Дата:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Ashesh Vashi
Дата:
On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features.
+1

-- Thanks, Ashesh 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Victoria Henry
Дата:
Hi Hackers,

We just reread the issue in Redmine, and our take is a little bit different. Looks like the problem the person is complaining the tabs sticking to the mouse when clicked. We also experience that problem while developing.

How hard would it be to eliminate the Drag and Drop of tabs in the current implementation?
Do we think this might be a problem for the majority of the users?

Thanks
Victoria & Joao

On Wed, Apr 4, 2018 at 10:41 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features.
+1

-- Thanks, Ashesh 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Dave Page
Дата:


On Wed, Apr 4, 2018 at 4:46 PM, Victoria Henry <vhenry@pivotal.io> wrote:
Hi Hackers,

We just reread the issue in Redmine, and our take is a little bit different. Looks like the problem the person is complaining the tabs sticking to the mouse when clicked. We also experience that problem while developing.

There isn't just one issue. Various people have asked for the ability to lock the view in the configuration they like.
 

How hard would it be to eliminate the Drag and Drop of tabs in the current implementation?
Do we think this might be a problem for the majority of the users?

When it comes to removing features, I don't really care about the majority. Creating annoyance or inconvenience by removing working, useful features for even a small percentage of users is not something I consider an option unless absolutely essential.
 

Thanks
Victoria & Joao

On Wed, Apr 4, 2018 at 10:41 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features.
+1

-- Thanks, Ashesh 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company





--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Khushboo Vashi
Дата:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Robert Eckhardt
Дата:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Murtuza Zabuawala
Дата:
Hi Dave,

Please find the updated patch, Now we are able to lock wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Вложения

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Dave Page
Дата:
Akshay, could you review/commit this please?

Please also update the release_notes_3_1.rst file when you commit user-visible changes, to make it easier to build the release notes.

Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Please find the updated patch, Now we are able to lock wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Akshay Joshi
Дата:


On Tue, Apr 24, 2018 at 1:17 PM, Dave Page <dpage@pgadmin.org> wrote:
Akshay, could you review/commit this please?

Please also update the release_notes_3_1.rst file when you commit user-visible changes, to make it easier to build the release notes.

   Sure 

Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Please find the updated patch, Now we are able to lock wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Akshay Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Joao De Almeida Pereira
Дата:
Hi Murtuza,

We tested the patch and everything looks fine.  We also refactors some parts to include things like strict equality and using let/const instead of var.  The updated patch is attached.
In the future, it will be more valuable to have the translation to ES6 and the feature work in separate commits so it is easier to understand what changed.

Sincerely,

Joao and Victoria



On Tue, Apr 24, 2018 at 4:58 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
On Tue, Apr 24, 2018 at 1:17 PM, Dave Page <dpage@pgadmin.org> wrote:
Akshay, could you review/commit this please?

Please also update the release_notes_3_1.rst file when you commit user-visible changes, to make it easier to build the release notes.

   Sure 

Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Please find the updated patch, Now we are able to lock wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Akshay Joshi
Sr. Software Architect


Вложения

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Joao De Almeida Pereira
Дата:
Hi,
Apparently the last version was not applying, here is the new version that should apply correctly

Thanks
Victoria & Joao

On Tue, Apr 24, 2018 at 10:56 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

We tested the patch and everything looks fine.  We also refactors some parts to include things like strict equality and using let/const instead of var.  The updated patch is attached.
In the future, it will be more valuable to have the translation to ES6 and the feature work in separate commits so it is easier to understand what changed.

Sincerely,

Joao and Victoria



On Tue, Apr 24, 2018 at 4:58 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
On Tue, Apr 24, 2018 at 1:17 PM, Dave Page <dpage@pgadmin.org> wrote:
Akshay, could you review/commit this please?

Please also update the release_notes_3_1.rst file when you commit user-visible changes, to make it easier to build the release notes.

   Sure 

Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Please find the updated patch, Now we are able to lock wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Akshay Joshi
Sr. Software Architect


Вложения

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Joao De Almeida Pereira
Дата:
haha,
Just joking, now we have a good version that passes tests and all.

We found out that a test was failing in the patch version 5:
HeadlessChrome 0.0.0 (Linux 0.0.0) Panel when we create a panel and user created panel without defining isMoveable then it should be moveable it should call moveable method with true as argument FAILED   Expected false to be true.       at UserContext.<anonymous> (regression/javascript/browser/panel_spec.js:12886:38)
To solve this problem we decided to change the Panel class to match what the test say.

Thanks
Victoria & Joao


On Tue, Apr 24, 2018 at 11:08 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi,
Apparently the last version was not applying, here is the new version that should apply correctly

Thanks
Victoria & Joao

On Tue, Apr 24, 2018 at 10:56 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

We tested the patch and everything looks fine.  We also refactors some parts to include things like strict equality and using let/const instead of var.  The updated patch is attached.
In the future, it will be more valuable to have the translation to ES6 and the feature work in separate commits so it is easier to understand what changed.

Sincerely,

Joao and Victoria



On Tue, Apr 24, 2018 at 4:58 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
On Tue, Apr 24, 2018 at 1:17 PM, Dave Page <dpage@pgadmin.org> wrote:
Akshay, could you review/commit this please?

Please also update the release_notes_3_1.rst file when you commit user-visible changes, to make it easier to build the release notes.

   Sure 

Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Please find the updated patch, Now we are able to lock wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Akshay Joshi
Sr. Software Architect


Вложения

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Murtuza Zabuawala
Дата:
Thank you Joao for reviewing.


On Tue, Apr 24, 2018 at 9:11 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
haha,
Just joking, now we have a good version that passes tests and all.

We found out that a test was failing in the patch version 5:
HeadlessChrome 0.0.0 (Linux 0.0.0) Panel when we create a panel and user created panel without defining isMoveable then it should be moveable it should call moveable method with true as argument FAILED   Expected false to be true.       at UserContext.<anonymous> (regression/javascript/browser/panel_spec.js:12886:38)
To solve this problem we decided to change the Panel class to match what the test say.

Thanks
Victoria & Joao


On Tue, Apr 24, 2018 at 11:08 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi,
Apparently the last version was not applying, here is the new version that should apply correctly

Thanks
Victoria & Joao

On Tue, Apr 24, 2018 at 10:56 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

We tested the patch and everything looks fine.  We also refactors some parts to include things like strict equality and using let/const instead of var.  The updated patch is attached.
In the future, it will be more valuable to have the translation to ES6 and the feature work in separate commits so it is easier to understand what changed.

Sincerely,

Joao and Victoria



On Tue, Apr 24, 2018 at 4:58 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
On Tue, Apr 24, 2018 at 1:17 PM, Dave Page <dpage@pgadmin.org> wrote:
Akshay, could you review/commit this please?

Please also update the release_notes_3_1.rst file when you commit user-visible changes, to make it easier to build the release notes.

   Sure 

Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Please find the updated patch, Now we are able to lock wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Akshay Joshi
Sr. Software Architect



Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Akshay Joshi
Дата:
Hi Joao/Murtuza

It break's the functionality, I am able to move "Data output", "Explain" etc.. panel of Query Tool, even if "Lock layout?" is set to True.
Apart from above I have found more issue. Below are the steps to reproduce:
  • Set the "Lock layout?" flag to False.
  • Move out Dashboard panel.
  • Set the "Lock layout?" flag to True.
  • Close the Dashboard panel, as layout is locked and empty Dashboard panel is still visible. (Refer attached screenshot)   

On Tue, Apr 24, 2018 at 9:11 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
haha,
Just joking, now we have a good version that passes tests and all.

We found out that a test was failing in the patch version 5:
HeadlessChrome 0.0.0 (Linux 0.0.0) Panel when we create a panel and user created panel without defining isMoveable then it should be moveable it should call moveable method with true as argument FAILED    Expected false to be true.        at UserContext.<anonymous> (regression/javascript/browser/panel_spec.js:12886:38)
To solve this problem we decided to change the Panel class to match what the test say.

Thanks
Victoria & Joao


On Tue, Apr 24, 2018 at 11:08 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi,
Apparently the last version was not applying, here is the new version that should apply correctly

Thanks
Victoria & Joao

On Tue, Apr 24, 2018 at 10:56 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

We tested the patch and everything looks fine.  We also refactors some parts to include things like strict equality and using let/const instead of var.  The updated patch is attached.
In the future, it will be more valuable to have the translation to ES6 and the feature work in separate commits so it is easier to understand what changed.

Sincerely,

Joao and Victoria



On Tue, Apr 24, 2018 at 4:58 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
On Tue, Apr 24, 2018 at 1:17 PM, Dave Page <dpage@pgadmin.org> wrote:
Akshay, could you review/commit this please?

Please also update the release_notes_3_1.rst file when you commit user-visible changes, to make it easier to build the release notes.

   Sure 

Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Please find the updated patch, Now we are able to lock wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Akshay Joshi
Sr. Software Architect





--
Akshay Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
Вложения

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Murtuza Zabuawala
Дата:
Hi Akshay,


On Wed, Apr 25, 2018 at 2:37 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Joao/Murtuza

It break's the functionality, I am able to move "Data output", "Explain" etc.. panel of Query Tool, even if "Lock layout?" is set to True.
 
​It's working properly in v5 patch, Something went wrong while refactoring.​
 

Apart from above I have found more issue. Below are the steps to reproduce:
  • Set the "Lock layout?" flag to False.
  • Move out Dashboard panel.
  • Set the "Lock layout?" flag to True.
  • Close the Dashboard panel, as layout is locked and empty Dashboard panel is still visible. (Refer attached screenshot)  
​That's because we have set the Panel moveable property to False, they won't auto resize, As discussed earlier if user drag any panel out of panel group it gets render in seprate wcFrame. I think that needs to be taken care by user before they decide to lock the layout, We can not expilcitly set panel's closeable property to False when layout is locked, If we do so user will not be able to close any Query tool, Debugger panels.​
 


On Tue, Apr 24, 2018 at 9:11 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
haha,
Just joking, now we have a good version that passes tests and all.

We found out that a test was failing in the patch version 5:
HeadlessChrome 0.0.0 (Linux 0.0.0) Panel when we create a panel and user created panel without defining isMoveable then it should be moveable it should call moveable method with true as argument FAILED   Expected false to be true.       at UserContext.<anonymous> (regression/javascript/browser/panel_spec.js:12886:38)
To solve this problem we decided to change the Panel class to match what the test say.

Thanks
Victoria & Joao


On Tue, Apr 24, 2018 at 11:08 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi,
Apparently the last version was not applying, here is the new version that should apply correctly

Thanks
Victoria & Joao

On Tue, Apr 24, 2018 at 10:56 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

We tested the patch and everything looks fine.  We also refactors some parts to include things like strict equality and using let/const instead of var.  The updated patch is attached.
In the future, it will be more valuable to have the translation to ES6 and the feature work in separate commits so it is easier to understand what changed.

Sincerely,

Joao and Victoria



On Tue, Apr 24, 2018 at 4:58 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
On Tue, Apr 24, 2018 at 1:17 PM, Dave Page <dpage@pgadmin.org> wrote:
Akshay, could you review/commit this please?

Please also update the release_notes_3_1.rst file when you commit user-visible changes, to make it easier to build the release notes.

   Sure 

Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Please find the updated patch, Now we are able to lock wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Akshay Joshi
Sr. Software Architect





--
Akshay Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Dave Page
Дата:
All,

We just had a brief discussion in our EDB sprint planning meeting about this. There is a non-zero chance that we're going to have to fork wcDocker in the near future, in order to update it to work with jQuery 3. If we do that, then it may be significantly easier to fix this issue in that fork (perhaps by adding a single lockLayout(bool) function, rather than trying to do so from pgAdmin.

I think (unless Murtuza believes that won't help), that we're better off holding on this for now until we know if we've had to do that.

On Wed, Apr 25, 2018 at 10:23 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Akshay,


On Wed, Apr 25, 2018 at 2:37 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Joao/Murtuza

It break's the functionality, I am able to move "Data output", "Explain" etc.. panel of Query Tool, even if "Lock layout?" is set to True.
 
​It's working properly in v5 patch, Something went wrong while refactoring.​
 

Apart from above I have found more issue. Below are the steps to reproduce:
  • Set the "Lock layout?" flag to False.
  • Move out Dashboard panel.
  • Set the "Lock layout?" flag to True.
  • Close the Dashboard panel, as layout is locked and empty Dashboard panel is still visible. (Refer attached screenshot)  
​That's because we have set the Panel moveable property to False, they won't auto resize, As discussed earlier if user drag any panel out of panel group it gets render in seprate wcFrame. I think that needs to be taken care by user before they decide to lock the layout, We can not expilcitly set panel's closeable property to False when layout is locked, If we do so user will not be able to close any Query tool, Debugger panels.​
 


On Tue, Apr 24, 2018 at 9:11 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
haha,
Just joking, now we have a good version that passes tests and all.

We found out that a test was failing in the patch version 5:
HeadlessChrome 0.0.0 (Linux 0.0.0) Panel when we create a panel and user created panel without defining isMoveable then it should be moveable it should call moveable method with true as argument FAILED   Expected false to be true.       at UserContext.<anonymous> (regression/javascript/browser/panel_spec.js:12886:38)
To solve this problem we decided to change the Panel class to match what the test say.

Thanks
Victoria & Joao


On Tue, Apr 24, 2018 at 11:08 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi,
Apparently the last version was not applying, here is the new version that should apply correctly

Thanks
Victoria & Joao

On Tue, Apr 24, 2018 at 10:56 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

We tested the patch and everything looks fine.  We also refactors some parts to include things like strict equality and using let/const instead of var.  The updated patch is attached.
In the future, it will be more valuable to have the translation to ES6 and the feature work in separate commits so it is easier to understand what changed.

Sincerely,

Joao and Victoria



On Tue, Apr 24, 2018 at 4:58 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
On Tue, Apr 24, 2018 at 1:17 PM, Dave Page <dpage@pgadmin.org> wrote:
Akshay, could you review/commit this please?

Please also update the release_notes_3_1.rst file when you commit user-visible changes, to make it easier to build the release notes.

   Sure 

Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Please find the updated patch, Now we are able to lock wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Akshay Joshi
Sr. Software Architect





--
Akshay Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246




--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Murtuza Zabuawala
Дата:
Hi Dave,

On Wed, Apr 25, 2018 at 3:36 PM, Dave Page <dpage@pgadmin.org> wrote:
All,

We just had a brief discussion in our EDB sprint planning meeting about this. There is a non-zero chance that we're going to have to fork wcDocker in the near future, in order to update it to work with jQuery 3. If we do that, then it may be significantly easier to fix this issue in that fork (perhaps by adding a single lockLayout(bool) function, rather than trying to do so from pgAdmin.

I think (unless Murtuza believes that won't help), that we're better off holding on this for now until we know if we've had to do that.

​I don't have any objection forking the code and adding the flag to lock the panel,  But I'm certain that 
we will use the same inbuilt method panel.moveable(false) which we have used right now in the patch to prevent a panel from floating and will face the same issue again which Akshay mentioned in his last email.

Let me know if you want me to attach latest patch onto the ticket for future reference and update the ticket accordingly​.


On Wed, Apr 25, 2018 at 10:23 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Akshay,


On Wed, Apr 25, 2018 at 2:37 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Joao/Murtuza

It break's the functionality, I am able to move "Data output", "Explain" etc.. panel of Query Tool, even if "Lock layout?" is set to True.
 
​It's working properly in v5 patch, Something went wrong while refactoring.​
 

Apart from above I have found more issue. Below are the steps to reproduce:
  • Set the "Lock layout?" flag to False.
  • Move out Dashboard panel.
  • Set the "Lock layout?" flag to True.
  • Close the Dashboard panel, as layout is locked and empty Dashboard panel is still visible. (Refer attached screenshot)  
​That's because we have set the Panel moveable property to False, they won't auto resize, As discussed earlier if user drag any panel out of panel group it gets render in seprate wcFrame. I think that needs to be taken care by user before they decide to lock the layout, We can not expilcitly set panel's closeable property to False when layout is locked, If we do so user will not be able to close any Query tool, Debugger panels.​
 


On Tue, Apr 24, 2018 at 9:11 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
haha,
Just joking, now we have a good version that passes tests and all.

We found out that a test was failing in the patch version 5:
HeadlessChrome 0.0.0 (Linux 0.0.0) Panel when we create a panel and user created panel without defining isMoveable then it should be moveable it should call moveable method with true as argument FAILED   Expected false to be true.       at UserContext.<anonymous> (regression/javascript/browser/panel_spec.js:12886:38)
To solve this problem we decided to change the Panel class to match what the test say.

Thanks
Victoria & Joao


On Tue, Apr 24, 2018 at 11:08 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi,
Apparently the last version was not applying, here is the new version that should apply correctly

Thanks
Victoria & Joao

On Tue, Apr 24, 2018 at 10:56 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

We tested the patch and everything looks fine.  We also refactors some parts to include things like strict equality and using let/const instead of var.  The updated patch is attached.
In the future, it will be more valuable to have the translation to ES6 and the feature work in separate commits so it is easier to understand what changed.

Sincerely,

Joao and Victoria



On Tue, Apr 24, 2018 at 4:58 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
On Tue, Apr 24, 2018 at 1:17 PM, Dave Page <dpage@pgadmin.org> wrote:
Akshay, could you review/commit this please?

Please also update the release_notes_3_1.rst file when you commit user-visible changes, to make it easier to build the release notes.

   Sure 

Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Please find the updated patch, Now we are able to lock wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Akshay Joshi
Sr. Software Architect





--
Akshay Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246




--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Dave Page
Дата:


On Wed, Apr 25, 2018 at 12:13 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Wed, Apr 25, 2018 at 3:36 PM, Dave Page <dpage@pgadmin.org> wrote:
All,

We just had a brief discussion in our EDB sprint planning meeting about this. There is a non-zero chance that we're going to have to fork wcDocker in the near future, in order to update it to work with jQuery 3. If we do that, then it may be significantly easier to fix this issue in that fork (perhaps by adding a single lockLayout(bool) function, rather than trying to do so from pgAdmin.

I think (unless Murtuza believes that won't help), that we're better off holding on this for now until we know if we've had to do that.

​I don't have any objection forking the code and adding the flag to lock the panel,  But I'm certain that 
we will use the same inbuilt method panel.moveable(false) which we have used right now in the patch to prevent a panel from floating and will face the same issue again which Akshay mentioned in his last email.

Let me know if you want me to attach latest patch onto the ticket for future reference and update the ticket accordingly​.

That's probably a good idea - thanks.
 


On Wed, Apr 25, 2018 at 10:23 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Akshay,


On Wed, Apr 25, 2018 at 2:37 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Joao/Murtuza

It break's the functionality, I am able to move "Data output", "Explain" etc.. panel of Query Tool, even if "Lock layout?" is set to True.
 
​It's working properly in v5 patch, Something went wrong while refactoring.​
 

Apart from above I have found more issue. Below are the steps to reproduce:
  • Set the "Lock layout?" flag to False.
  • Move out Dashboard panel.
  • Set the "Lock layout?" flag to True.
  • Close the Dashboard panel, as layout is locked and empty Dashboard panel is still visible. (Refer attached screenshot)  
​That's because we have set the Panel moveable property to False, they won't auto resize, As discussed earlier if user drag any panel out of panel group it gets render in seprate wcFrame. I think that needs to be taken care by user before they decide to lock the layout, We can not expilcitly set panel's closeable property to False when layout is locked, If we do so user will not be able to close any Query tool, Debugger panels.​
 


On Tue, Apr 24, 2018 at 9:11 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
haha,
Just joking, now we have a good version that passes tests and all.

We found out that a test was failing in the patch version 5:
HeadlessChrome 0.0.0 (Linux 0.0.0) Panel when we create a panel and user created panel without defining isMoveable then it should be moveable it should call moveable method with true as argument FAILED   Expected false to be true.       at UserContext.<anonymous> (regression/javascript/browser/panel_spec.js:12886:38)
To solve this problem we decided to change the Panel class to match what the test say.

Thanks
Victoria & Joao


On Tue, Apr 24, 2018 at 11:08 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi,
Apparently the last version was not applying, here is the new version that should apply correctly

Thanks
Victoria & Joao

On Tue, Apr 24, 2018 at 10:56 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

We tested the patch and everything looks fine.  We also refactors some parts to include things like strict equality and using let/const instead of var.  The updated patch is attached.
In the future, it will be more valuable to have the translation to ES6 and the feature work in separate commits so it is easier to understand what changed.

Sincerely,

Joao and Victoria



On Tue, Apr 24, 2018 at 4:58 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
On Tue, Apr 24, 2018 at 1:17 PM, Dave Page <dpage@pgadmin.org> wrote:
Akshay, could you review/commit this please?

Please also update the release_notes_3_1.rst file when you commit user-visible changes, to make it easier to build the release notes.

   Sure 

Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Please find the updated patch, Now we are able to lock wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Akshay Joshi
Sr. Software Architect





--
Akshay Joshi
Sr. Software Architect


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246




--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company




--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Joao De Almeida Pereira
Дата:
Hi,
@Murtuza: We didn't notice the issue, can you please advise on what need to change to make it work? The only change we did was to make one test pass.

@Hackers: In our point of view it is never good to fork a library. But if he really have to do it, then we should fork it in Github, make our code accessible to other people, and we should add it as a dependency on package.json


Thanks
Anthony & Joao


On Wed, Apr 25, 2018 at 7:14 AM Dave Page <dpage@pgadmin.org> wrote:
On Wed, Apr 25, 2018 at 12:13 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Wed, Apr 25, 2018 at 3:36 PM, Dave Page <dpage@pgadmin.org> wrote:
All,

We just had a brief discussion in our EDB sprint planning meeting about this. There is a non-zero chance that we're going to have to fork wcDocker in the near future, in order to update it to work with jQuery 3. If we do that, then it may be significantly easier to fix this issue in that fork (perhaps by adding a single lockLayout(bool) function, rather than trying to do so from pgAdmin.

I think (unless Murtuza believes that won't help), that we're better off holding on this for now until we know if we've had to do that.

​I don't have any objection forking the code and adding the flag to lock the panel,  But I'm certain that 
we will use the same inbuilt method panel.moveable(false) which we have used right now in the patch to prevent a panel from floating and will face the same issue again which Akshay mentioned in his last email.

Let me know if you want me to attach latest patch onto the ticket for future reference and update the ticket accordingly​.

That's probably a good idea - thanks.
 


On Wed, Apr 25, 2018 at 10:23 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Akshay,


On Wed, Apr 25, 2018 at 2:37 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Joao/Murtuza

It break's the functionality, I am able to move "Data output", "Explain" etc.. panel of Query Tool, even if "Lock layout?" is set to True.
 
​It's working properly in v5 patch, Something went wrong while refactoring.​
 

Apart from above I have found more issue. Below are the steps to reproduce:
  • Set the "Lock layout?" flag to False.
  • Move out Dashboard panel.
  • Set the "Lock layout?" flag to True.
  • Close the Dashboard panel, as layout is locked and empty Dashboard panel is still visible. (Refer attached screenshot)  
​That's because we have set the Panel moveable property to False, they won't auto resize, As discussed earlier if user drag any panel out of panel group it gets render in seprate wcFrame. I think that needs to be taken care by user before they decide to lock the layout, We can not expilcitly set panel's closeable property to False when layout is locked, If we do so user will not be able to close any Query tool, Debugger panels.​
 


On Tue, Apr 24, 2018 at 9:11 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
haha,
Just joking, now we have a good version that passes tests and all.

We found out that a test was failing in the patch version 5:
HeadlessChrome 0.0.0 (Linux 0.0.0) Panel when we create a panel and user created panel without defining isMoveable then it should be moveable it should call moveable method with true as argument FAILED   Expected false to be true.       at UserContext.<anonymous> (regression/javascript/browser/panel_spec.js:12886:38)
To solve this problem we decided to change the Panel class to match what the test say.

Thanks
Victoria & Joao


On Tue, Apr 24, 2018 at 11:08 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi,
Apparently the last version was not applying, here is the new version that should apply correctly

Thanks
Victoria & Joao

On Tue, Apr 24, 2018 at 10:56 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

We tested the patch and everything looks fine.  We also refactors some parts to include things like strict equality and using let/const instead of var.  The updated patch is attached.
In the future, it will be more valuable to have the translation to ES6 and the feature work in separate commits so it is easier to understand what changed.

Sincerely,

Joao and Victoria



On Tue, Apr 24, 2018 at 4:58 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
On Tue, Apr 24, 2018 at 1:17 PM, Dave Page <dpage@pgadmin.org> wrote:
Akshay, could you review/commit this please?

Please also update the release_notes_3_1.rst file when you commit user-visible changes, to make it easier to build the release notes.

   Sure 

Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Please find the updated patch, Now we are able to lock wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Akshay Joshi
Sr. Software Architect





--
Akshay Joshi
Sr. Software Architect






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company




--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

От
Murtuza Zabuawala
Дата:
Hi Joao,

On Wed, Apr 25, 2018 at 6:55 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi,
@Murtuza: We didn't notice the issue, can you please advise on what need to change to make it work? The only change we did was to make one test pass.
 
I moved to another project so I didn't get a chance to look into the code but a
s you are aware that ​we are no longer considering given the patch as a fix for the issue instead someone from the team might fork the ​
 
​code and add the option in the library itself.​

Regards,
Murtuza


@Hackers: In our point of view it is never good to fork a library. But if he really have to do it, then we should fork it in Github, make our code accessible to other people, and we should add it as a dependency on package.json


Thanks
Anthony & Joao


On Wed, Apr 25, 2018 at 7:14 AM Dave Page <dpage@pgadmin.org> wrote:
On Wed, Apr 25, 2018 at 12:13 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Wed, Apr 25, 2018 at 3:36 PM, Dave Page <dpage@pgadmin.org> wrote:
All,

We just had a brief discussion in our EDB sprint planning meeting about this. There is a non-zero chance that we're going to have to fork wcDocker in the near future, in order to update it to work with jQuery 3. If we do that, then it may be significantly easier to fix this issue in that fork (perhaps by adding a single lockLayout(bool) function, rather than trying to do so from pgAdmin.

I think (unless Murtuza believes that won't help), that we're better off holding on this for now until we know if we've had to do that.

​I don't have any objection forking the code and adding the flag to lock the panel,  But I'm certain that 
we will use the same inbuilt method panel.moveable(false) which we have used right now in the patch to prevent a panel from floating and will face the same issue again which Akshay mentioned in his last email.

Let me know if you want me to attach latest patch onto the ticket for future reference and update the ticket accordingly​.

That's probably a good idea - thanks.
 


On Wed, Apr 25, 2018 at 10:23 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Akshay,


On Wed, Apr 25, 2018 at 2:37 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Joao/Murtuza

It break's the functionality, I am able to move "Data output", "Explain" etc.. panel of Query Tool, even if "Lock layout?" is set to True.
 
​It's working properly in v5 patch, Something went wrong while refactoring.​
 

Apart from above I have found more issue. Below are the steps to reproduce:
  • Set the "Lock layout?" flag to False.
  • Move out Dashboard panel.
  • Set the "Lock layout?" flag to True.
  • Close the Dashboard panel, as layout is locked and empty Dashboard panel is still visible. (Refer attached screenshot)  
​That's because we have set the Panel moveable property to False, they won't auto resize, As discussed earlier if user drag any panel out of panel group it gets render in seprate wcFrame. I think that needs to be taken care by user before they decide to lock the layout, We can not expilcitly set panel's closeable property to False when layout is locked, If we do so user will not be able to close any Query tool, Debugger panels.​
 


On Tue, Apr 24, 2018 at 9:11 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
haha,
Just joking, now we have a good version that passes tests and all.

We found out that a test was failing in the patch version 5:
HeadlessChrome 0.0.0 (Linux 0.0.0) Panel when we create a panel and user created panel without defining isMoveable then it should be moveable it should call moveable method with true as argument FAILED   Expected false to be true.       at UserContext.<anonymous> (regression/javascript/browser/panel_spec.js:12886:38)
To solve this problem we decided to change the Panel class to match what the test say.

Thanks
Victoria & Joao


On Tue, Apr 24, 2018 at 11:08 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi,
Apparently the last version was not applying, here is the new version that should apply correctly

Thanks
Victoria & Joao

On Tue, Apr 24, 2018 at 10:56 AM Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hi Murtuza,

We tested the patch and everything looks fine.  We also refactors some parts to include things like strict equality and using let/const instead of var.  The updated patch is attached.
In the future, it will be more valuable to have the translation to ES6 and the feature work in separate commits so it is easier to understand what changed.

Sincerely,

Joao and Victoria



On Tue, Apr 24, 2018 at 4:58 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
On Tue, Apr 24, 2018 at 1:17 PM, Dave Page <dpage@pgadmin.org> wrote:
Akshay, could you review/commit this please?

Please also update the release_notes_3_1.rst file when you commit user-visible changes, to make it easier to build the release notes.

   Sure 

Thanks.

On Tue, Apr 24, 2018 at 8:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Please find the updated patch, Now we are able to lock wcFrame and wcPanel both.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Thu, Apr 5, 2018 at 6:32 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Wed, Apr 4, 2018 at 11:31 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:


On Wed, Apr 4, 2018 at 8:09 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 12:54 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 5:00 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 10:45 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
On Wed, Apr 4, 2018 at 2:47 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Apr 4, 2018 at 7:20 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

On Tue, Apr 3, 2018 at 9:03 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 3, 2018 at 12:56 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

Thanks Joao for reviewing.

PFA updated patch.

On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello,

On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

​Hello,

Please find updated patch, 

Now layout will be locked after user updates its preferences, w
e have used ​
templated variable in the javascript file
​ because we do not have preference module or preference cache available when the page loads and panels gets rendered, 
​I
​ also 
made changes in JS tests as per Joao's review comments.
Looks like everything is working when we change the lock.
As a personal preferences I would prefer to see this in at least 2 commits, one that is related to the preference issue and another one that is related to this story.


All the tests are working, but he linter is failing:
/tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
1 E303 too many blank lines (2) 
1 
​Fixed​
 


@Dave/Pivotal team,
The given patch is working fine for all the Tabs/Panels (all the panels from main window as well as from Query tool and Debugger) but I'm facing an issue while handling the Browser tree section, It is a wcDocer frame and not a wcDocker panel. Like wcDocker panel, wcDocker frame do not provide any API so that a developer can prevent drag-drop functionality on it.

It's not working fine for me. For example, if I put the SQL Panel on it's own below the properties/stats panels (so it looks like pgAdmin 3 used to by default), and then lock the layout, I can un-dock the SQL panel into a dialogue, but then cannot re-dock it. I can do weird things with the browser tree as well, probably because it's a frame as you say.
 
​That is expected behaviour ​because once you drag the panel out of the group of Panels then it becomes individual Frame, That is what the author of the wcDocker replied on my question, 
"A panel must either be initialized as movable or non-movable from the beginning and never changed because it generates a different arrangement of elements depending. This feature should only ever be used within the onCreate method of the panel. I should probably have been more clear about this limitation in the documentation."


So does it become a panel again if a second panel is added to the new tab group?
​No, it stays Frame.​
 
As far as I understand Panel needs a Frame to render itself if it is not attached to the main docker instance.​

There must be some way we can lock a tab that's not part of a group.
At a moment there is no way of ​
locking frames out of the box :(

Hmm, so the question becomes: do we include the lock feature, but rename it to "Lock Tabs" or something similar, or leave it out altogether? It clearly doesn't do everything we want right now. 
​I would say lets include the feature by adding warning note that this feature works with default layout only, And I don't think most user will try to drag drop Browser panel ​
anyway, meanwhile I'll check what changes are required in main source code to make the Frame lock.

Anyone else have any thoughts on this? Personally I don't like including half-baked features. 

+1, but we need to find out the way as this feature is requested by many users. 

100% agree. I can convince my self that this feature request has to do with locking panels into a certain layout. I can also convince myself that the same request is simple because users are frustrated with the fact that the tabs and panes move around and they find that behavior annoying. 

-- Rob
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Akshay Joshi
Sr. Software Architect





--
Akshay Joshi
Sr. Software Architect






--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company




--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company