Search for CTF/Task #129
Search for CTF/Task #129
Conversation
Only the files in /src where formated with `yarn format` now the subfolder are also formated. + formating importCtf and savepointWrapper
Open the dialog from a button in the menu bar
|
This is how it looks like so far.
|
Better style of search result task item
JJ-8
left a comment
JJ-8
left a comment
There was a problem hiding this comment.
Can you sync this branch with the main branch? There is now a conflict with migration 39:
Error: Migration failed. Reason: Hashes don't match for migrations '39-search-ctfs.sql'.
This is because of the 39-sort-users.sql migration already existing.
|
We should use https://github.com/graphile-contrib/postgraphile-plugin-connection-filter instead of our own search function, this could also replace the incomming and pastctf functions. |
|
@JJ-8 Are u comparing with the dev branch ? I can't see the "39-sort-users.sql" migration. |
|
No, I think it is only merged to the main branch. So that's why it is not in dev. You can find the file here: https://github.com/TFNS/CTFNote/blob/main/api/migrations/39-sort-users.sql |
|
@SakiiR, are you planning to add the |
JJ-8
left a comment
JJ-8
left a comment
There was a problem hiding this comment.
Thank you for the quick update @SakiiR! Now I was able to install and test it locally. I have some remarks about the functionality, see above for the details.
But overall: great work! I really like the quick navigation! :)
@XeR, I am sure you also have some remarks on the code, functionality, etc. Can you please also review this? Or if everything is alright, then we just merge.
Moved to hotkeys-js to handle hotkeys Moved the composable in the MainLayout to handle authenticated status Typos
|
Please, can you make some tests with the new hotkeys handling ? I tested my self on OSX and it feels ok =) |
JJ-8
left a comment
JJ-8
left a comment
There was a problem hiding this comment.
Thank you for the update! The shortcuts now partially work. I don't think it is possible to overwrite browser-defined shortcuts like ctrl+n but if that works on other operating systems, then it is perfectly fine by me. At least up, down and ctrl+k do work for me.
I have two additional remarks however:
- The searching is case sensitive. This is quite annoying.
- The
postgraphile-plugin-connection-filterpackage has Performance and Security remarks that are not taken into consideration at the moment. I think this should be restricted.
JJ-8
left a comment
JJ-8
left a comment
There was a problem hiding this comment.
Great work! I think these are the final comments and then it can be merged. I have already supplied patches for the bugs, so you only have to confirm it :)
Unbinding arrows shortcuts when closing the popup
JJ-8
left a comment
JJ-8
left a comment
There was a problem hiding this comment.
Thank you very much for your contribution!
3 participants
cf #80