public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nikhil Mohite <[email protected]>
To: Akshay Joshi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin][RM-7149]: [React] Port preferences dialog to React.
Date: Tue, 29 Mar 2022 15:35:55 +0530
Message-ID: <CAOBg0APEg6BZ9a6KV8EVJOFbDmx8tmsbdcz6i6TB6E2P1+jRMA@mail.gmail.com> (raw)
In-Reply-To: <CAOBg0AMYtPxsdnPPWPi8MSEMFPtmNeTJZa=h-TatSOSkKEXjkA@mail.gmail.com>
References: <CAOBg0AMfBEFesek3Uoet9zuNbHP5xA8OzWNGkP8JTt5CrGk9_w@mail.gmail.com>
<CANxoLDcivDBj9=E5q45DEF6f3iczQNZDT3wEOVH=QHGsUD8vUA@mail.gmail.com>
<CAOBg0APBNYjYzguSmW4_LAyPib+SFUDHwBv74-AT5UeWBw4rjQ@mail.gmail.com>
<CANxoLDe5Z0EKagvCYC6bSag4cb3SG5hJRzBegrz2cXgQCGmhJA@mail.gmail.com>
<CAOBg0AMYtPxsdnPPWPi8MSEMFPtmNeTJZa=h-TatSOSkKEXjkA@mail.gmail.com>
Hi Akshay,
PFA updated patch resolved the following issues in the preferences dialog:
1. Show a single note in the binary path.
2. Resolved issue in the keyboard shortcut.
On Wed, Mar 23, 2022 at 12:14 PM Nikhil Mohite <
[email protected]> wrote:
> Hi Akshay,
>
> Please find the updated patch, resolved the following issues in the
> preferences dialog.
>
> 1. Add server mode validation in the binary path.
> 2. Updated preferences tree rendering to avoid using the ReactDOM
> render.
> 3. Updated CSS for keyboard shortcuts checkbox border makes it
> consistent with input box border.
> 4. Fixed jasmine test case and improved code coverage.
> 5. Fixed sonarqube issues.
> 6. Added validation to disable "*Maximum column with" option *if "*Column
> **sized by" option *is set to *"Column name"* in *Query Tool -> Result
> grid*.
> 7. Updated documentation with the latest screenshots.
> 8. Updated documentation to fix issue #7261
> <https://redmine.postgresql.org/issues/7261;: Typo on the document.
>
>
>
> On Mon, Mar 21, 2022 at 1:31 PM Akshay Joshi <
> [email protected]> wrote:
>
>> Thanks, the patch applied. Screenshots are still not the latest. Take all
>> the screenshots with the latest changes and send them back.
>>
>> On Mon, Mar 21, 2022 at 10:00 AM Nikhil Mohite <
>> [email protected]> wrote:
>>
>>> Hi Akshay,
>>>
>>> PFA patch v2 resolved all the review comments.
>>>
>>> On Wed, Mar 9, 2022 at 5:48 PM Akshay Joshi <
>>> [email protected]> wrote:
>>>
>>>> Hi Nikhil
>>>>
>>>> Below are the review comments:
>>>> *GUI:*
>>>>
>>>> - By default, all nodes should be expanded, and at least one node
>>>> should be selected (Check existing behavior).
>>>> - Reduce the space between the expand/collapse (>) icon and the
>>>> tress node label. It is most likely icons that are taking up that space.
>>>> - Unable to select parent node. With old behavior when we select
>>>> the parent node its immediate first child is selected and the appropriate
>>>> page is displayed on the right side.
>>>> - The maximize button should be consistent with all other dialogs.
>>>> - Maximize not working properly, move the dialog bit up, and then
>>>> click Maximize button, dialog is not visible correctly.
>>>> - The help button is missing at the bottom.
>>>> - A documentation update is missing.
>>>> - Keyboard Shortcuts (Browser, Debugger, Query Tool, and ERD Tool) *Fix
>>>> for all*
>>>> - Between the 'Key' label and control, there should be a margin.
>>>> - Reduce the width of the key control.
>>>> - The key control loses focus when the key is pressed. Verify
>>>> old behavior.
>>>> - Nodes: Can we add a message or label at the top about settings?
>>>> Previously, we had a switch control with a 'Show/Hide' label that was clear
>>>> to the end-user.
>>>> - Dashboard -> Display -> Long-running query thresholds:
>>>> - Between the 'Warning'/'Alert' label and their control, there
>>>> should be a margin.
>>>> - There is no '*in minutes*' label at the end.
>>>> - Miscellaneous -> Themes: Themes preview is not visible.
>>>> - Binary Paths:
>>>> - 'Set as default' should be disabled until a path is provided.
>>>> - 'Set as default' must be a radio button, only one binary path
>>>> can be set to default not all.
>>>> - Unable to select binary path as select dialog is hidden behind
>>>> the preferences dialog. It seems the preferences dialog is the modal dialog.
>>>> - Adjust the validation icon properly, space should not be there
>>>> at the end.
>>>> - [image: Screenshot 2022-03-09 at 4.34.08 PM.png]
>>>> - Query Tool: CSV/TXT Output node should be after Auto completion.
>>>> - Browser -> Tab settings and Storage -> Options have identical
>>>> right side page. Remove the controls appropriately.
>>>>
>>>> I tried but was unable to reproduce the issue.
>>>
>>>>
>>>> *Code:*
>>>>
>>>> - Add comments/function header for all the new functions created.
>>>>
>>>>
>>>> On Mon, Mar 7, 2022 at 5:54 PM Nikhil Mohite <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi Hackers,
>>>>>
>>>>> Please find attached the patch for RM-7149
>>>>> <https://redmine.postgresql.org/issues/7149;: [React] Port
>>>>> preferences dialog to React.
>>>>>
>>>>> --
>>>>> *Thanks & Regards,*
>>>>> *Nikhil Mohite*
>>>>> *Senior Software Engineer.*
>>>>> *EDB Postgres* <https://www.enterprisedb.com/;
>>>>> *Mob.No: +91-7798364578.*
>>>>>
>>>>
>>>>
>>>> --
>>>> *Thanks & Regards*
>>>> *Akshay Joshi*
>>>> *pgAdmin Hacker | Principal Software Architect*
>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>
>>>> *Mobile: +91 976-788-8246*
>>>>
>>>
>>> Regards,
>>> Nikhil Mohite
>>>
>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Principal Software Architect*
>> *EDB Postgres <http://edbpostgres.com>*
>>
>> *Mobile: +91 976-788-8246*
>>
> Regards,
> Nikhil Mohite
>
Regards,
Nikhil Mohite
Attachments:
[image/png] Screenshot 2022-03-09 at 4.34.08 PM.png (10.3K, 3-Screenshot%202022-03-09%20at%204.34.08%20PM.png)
download | view image
[application/octet-stream] RM-7149_v4.patch (4.6K, 4-RM-7149_v4.patch)
download | inline diff:
diff --git a/web/pgadmin/preferences/static/js/components/PreferencesComponent.jsx b/web/pgadmin/preferences/static/js/components/PreferencesComponent.jsx
index bc238179..07415733 100644
--- a/web/pgadmin/preferences/static/js/components/PreferencesComponent.jsx
+++ b/web/pgadmin/preferences/static/js/components/PreferencesComponent.jsx
@@ -233,6 +233,7 @@ export default function PreferencesComponent({ ...props }) {
});
}, []);
function setPreferences(node, subNode, nodeData, preferencesValues, preferencesData) {
+ let addBinaryPathNote = false;
subNode.preferences.forEach((element) => {
let note = '';
let type = getControlMappedForType(element.type);
@@ -247,7 +248,10 @@ export default function PreferencesComponent({ ...props }) {
element.editable = false;
element.disabled = true;
preferencesValues[element.id] = JSON.parse(element.value);
- addNote(node, subNode, nodeData, preferencesData, note);
+ if(addBinaryPathNote) {
+ addNote(node, subNode, nodeData, preferencesData, note);
+ }
+ addBinaryPathNote = true;
}
else if (type == 'select') {
setControlProps(element);
diff --git a/web/pgadmin/static/js/components/KeyboardShortcuts.jsx b/web/pgadmin/static/js/components/KeyboardShortcuts.jsx
index 61661d43..688045b0 100644
--- a/web/pgadmin/static/js/components/KeyboardShortcuts.jsx
+++ b/web/pgadmin/static/js/components/KeyboardShortcuts.jsx
@@ -60,7 +60,7 @@ export default function KeyboardShortcuts({ value, onChange, fields }) {
const onCtrlChange = (e) => {
let newVal = { ...value };
- newVal.ctrl = e.target.checked;
+ newVal.control = e.target.checked;
onChange(newVal);
};
@@ -105,7 +105,7 @@ export default function KeyboardShortcuts({ value, onChange, fields }) {
} else if (element.name == 'control') {
return <Grid item lg={2} md={2} sm={2} xs={12} className={classes.inputLabel} key={_.uniqueId('c')}>
<Box className={classes.inputCheckboxClass}>
- <InputCheckbox id={ctrlCid} helpid={ctrlhelpid} value={value?.ctrl}
+ <InputCheckbox id={ctrlCid} helpid={ctrlhelpid} value={value?.control}
controlProps={ctrlProps}
onChange={onCtrlChange}></InputCheckbox>
</Box>
diff --git a/web/regression/javascript/components/KeyboardShortcuts.spec.js b/web/regression/javascript/components/KeyboardShortcuts.spec.js
index 370ce65d..c45d0be8 100644
--- a/web/regression/javascript/components/KeyboardShortcuts.spec.js
+++ b/web/regression/javascript/components/KeyboardShortcuts.spec.js
@@ -22,7 +22,7 @@ import { InputCheckbox } from '../../../pgadmin/static/js/components/FormCompone
describe('KeyboardShortcuts', () => {
let mount;
let defult_value = {
- 'ctrl': true,
+ 'control': true,
'alt': true,
'key': {
'char': 'a',
@@ -85,26 +85,26 @@ describe('KeyboardShortcuts', () => {
it('Key change', (done) => {
ctrl.find(OutlinedInput).at(0).find('input').simulate('keydown', { key: '', keyCode: 32});
- expect(onChange).toHaveBeenCalledWith({ ctrl: true, alt: true, key: { char: 'Space', key_code: 32 }, shift: false });
+ expect(onChange).toHaveBeenCalledWith({ control: true, alt: true, key: { char: 'Space', key_code: 32 }, shift: false });
done();
});
it('Shift option', (done) => {
ctrl.find(InputCheckbox).at(0).find('input').simulate('change', { target: { checked: true, name: 'shift' } });
- expect(onChange).toHaveBeenCalledWith({ ctrl: true, alt: true, key: { char: 'a', key_code: 97 }, shift: true });
+ expect(onChange).toHaveBeenCalledWith({ control: true, alt: true, key: { char: 'a', key_code: 97 }, shift: true });
done();
});
- it('Ctrl option', (done) => {
+ it('Control option', (done) => {
ctrl.find(InputCheckbox).at(1).find('input').simulate('change', { target: { checked: false, name: 'ctrl' } });
- expect(onChange).toHaveBeenCalledWith({ ctrl: false, alt: true, key: { char: 'a', key_code: 97 }, shift: false });
+ expect(onChange).toHaveBeenCalledWith({ control: false, alt: true, key: { char: 'a', key_code: 97 }, shift: false });
done();
});
it('Alt option', (done) => {
ctrl.find(InputCheckbox).at(2).find('input').simulate('change', { target: { checked: false, name: 'alt' } });
- expect(onChange).toHaveBeenCalledWith({ ctrl: true, alt: false, key: { char: 'a', key_code: 97 }, shift: false });
+ expect(onChange).toHaveBeenCalledWith({ control: true, alt: false, key: { char: 'a', key_code: 97 }, shift: false });
done();
});
view thread (7+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: [pgAdmin][RM-7149]: [React] Port preferences dialog to React.
In-Reply-To: <CAOBg0APEg6BZ9a6KV8EVJOFbDmx8tmsbdcz6i6TB6E2P1+jRMA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox