Add advanced search options to user profile#4012
Conversation
c266da1 to
43bd307
Compare
mathjazz
left a comment
There was a problem hiding this comment.
The changes made in the frontend should also be stored in the DB, otherwise the behaviour is inconsistent with how editor settings work.
I haven't looked at all the code yet.
Not sure I agree on this? That behavior would be very annoying for these settings. For example: I want to include IDs by default. For a specific search, I go in the menu and disable them. I shouldn't have to then click the menu again to restore my default preference. Also, I think the menu should reflect the current options coming from the URL (e.g. |
This is not an ideal UX. If you click the wrong button, or just want to switch between options, the interaction cost to make a change is high. I would keep the current behaviour and possibly restore the button.
I like the restore to default entry. But you don't see what the defaults are if you click on it, so triggerring search immediatelly is (like above) not the best UX. |
If clicking an option doesn't trigger the search, I can't see a way without the button. If I click around with the mouse, I don't want to have to click in the field and then switch to keyboard to press enter to start the search. |
|
Note: still trying to tidy things up, because I feel like I might have introduced unnecessary duplication or complexity through all the back and forth. Will re-flag for review once ready and tested. |
mathjazz
left a comment
There was a problem hiding this comment.
Nice job - it works great.
Left a couple of comments.
Please apply these changes to the CSS and then I'll take another pass at the frontend code.
/* translate.css | http://localhost:8000/static/translate.css */
.search-panel {
& .search-button {
width: 100%;
line-height: 22px;
padding: 3px;
}
& .menu {
& li {
&.action-item {
/* padding: 4px 2px; */
padding: 2px 4px;
}
/* padding: 4px 2px; */
padding: 2px 4px;
line-height: 22px;
}
}
& .visibility-switch {
& .fa-caret-down {
/* transform: translate(35%, -25%); */
transform: translate(40%, -25%);
}
}
}
& .fas {
/* margin-left: 1px; */
margin-left: -5px;
}
mathjazz
left a comment
There was a problem hiding this comment.
LGTM
Seems like these changes are not applied yet. Since they are not directly related to the PR, I can also land them separately:
/* translate.css | http://localhost:8000/static/translate.css */
.search-panel {
& .visibility-switch {
& .fa-caret-down {
/* transform: translate(35%, -25%); */
transform: translate(40%, -25%);
}
}
}
& .fas {
/* margin-left: 1px; */
margin-left: -5px;
}
I didn't scroll down 🤦🏼 |
|
Updated CSS, let me know if I missed anything else. |
Co-authored-by: Matjaž Horvat <matjaz.horvat@gmail.com>
|
That's what you just fixed in your latest attempt :) |



Fixes #4008
The Python part feels straightforward, the front-end not so much (likely because of my lack of love for JS and React).
One surprise was the amount of places where we hardcoded a default false value for these settings. I tried to define them in one place, so we can change them more easily in the future (well, one for Python and one for JS).
Profile settings are also used on the new /search page.