Re: [pgAdmin4][RM#2900] Adding accessibility features in query tool

Поиск
Список
Период
Сортировка
От Joao De Almeida Pereira
Тема Re: [pgAdmin4][RM#2900] Adding accessibility features in query tool
Дата
Msg-id CAE+jjak70+e4n-3ChhMC-iqk=cCNH=bVAgUo9tjeN+KYwUcWtg@mail.gmail.com
обсуждение исходный текст
Ответ на [pgAdmin4][RM#2900] Adding accessibility features in query tool  (Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com>)
Ответы Re: [pgAdmin4][RM#2900] Adding accessibility features in query tool
Список pgadmin-hackers
Hello Murtuza,
After reviewing the code I have some suggestions:
 - We should split the PEP and the features into different patches, or else it becomes very hard to separate Feature code from Code Style change
 - The function: `getTextRepresentaionShortcut` has a typo
 - As a personal point I have a hard time reading multiple declarations of variables in javascript under a single `var`, I prefer 1 `let` per variable
 - I think we should use cameCase for our variables in Javascript code
 - What is the reason behind the move of the key shortcuts back into the SQLEditorController? This look like something that could be extracted from SQLEditor file into its own
 - Looks like we are missing some test coverage on the new implemented parts
 - The getKeyboardShortcuts function is retrieving information from window.top and window.opener I think that we can come up with a better pattern then this. Relying on window information doesn't look very good. That was the pattern for JQuery that the new frameworks are trying to avoid because polluting the global scope is almost always a Code Smell.


I see there was some refactoring on the backend with the creation of RegisterQueryToolPreferences and it is looking good, hope that we can do this in more places specially in the front-end.

Using the patch that you sent I passed it through our CI and everything is Passing.

Thanks
Joao

On Tue, Feb 20, 2018 at 1:13 PM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA patch for adding accessibility features in query tool.
RM#2900

Please review.

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

В списке pgadmin-hackers по дате отправления:

Предыдущее
От: Harshal Dhumal
Дата:
Сообщение: Re: RM2898 Keyboard navigation in dialog tabs (nav tabs)
Следующее
От: Joao De Almeida Pereira
Дата:
Сообщение: [pgadmin4] Priorities of features