-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Viewer redesign #510
base: develop
Are you sure you want to change the base?
Viewer redesign #510
Conversation
* Adds tests for /users/ API * Adds a test setup using Keycloak for local authentication testing
* Also fix a possible crash when the configuration request fails
Documentation using Postman
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of having all viewers in 1 place instead of split over multiple tabs.
It would be nice if these viewers could have variable dimensions instead of having to fit into a grid view (e.g 1 column x 2 rows, 3 columns x 1 row...). This will potentially add a lot of complexity, so not sure if this should be a priority
@@ -30,7 +30,7 @@ const ProjectView: React.FC<ProjectViewProps> = (props) => { | |||
const [displayUpload, setDisplayUpload] = useState(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a second dataset doesn't seem to work. This issue is not caused by changes in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot reproduce this. Can you be more detailed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just try to add a second dataset but it won't show me the popup to upload the loom file and give the dataset a name.
I can replicate this in 2 ways (across 2 browsers):
- create project 1 (OK) -> add dataset 1 (OK) -> add dataset 2 (no popup)
- create project 1 (OK) -> add dataset 1 (OK) -> create project 2 (OK) -> add dataset to project 2 (no popup)
} | ||
})} | ||
|
||
{state.remove ? ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something is not right with the tracking of the remove state.
Steps to reproduce: create a grid of 2x2 and remove the first row or column. The buttons at the sides should be green but they remain (inactive) red.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cannot reproduce. Is there some chance you let your finger off the 'Shift' button outside the browser window?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me, if I hold the shift key, they buttons flicker between red and green, sometimes remaining on red, sometimes on green.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a weird browser quirk.
In chrome everything just works as expected.
In firefox, the button just stays red after removing the first row/column even after releasing the shift key.
The state can be fixed by pressing ctrl, but this should not be necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested on Firefox 97.0, Firefox 78.15.0esr, Chromium 90.0.4430.212 (Developer Build), and Gnome Web 3.38.2. I cannot re-produce this. What browser(s) and browser version(s) are you using? Can you reproduce with all extensions disabled (safe mode)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested with following browsers/versions:
Firefox 97.0.2 (Fedora 35): button stays red after removing first row/column and releasing shift
Firefox 98.0 (Fedora 35): button stays red after removing first row/column and releasing shift
Google Chrome 99.0.4844.51 (Fedora 35): works fine
Firefox 98.0.1 (Windows 10): buttons flicker. Releasing shift can either end in red or green
Google Chrome (Windows 10): buttons flicker again
I tried everything with and without my extensions enabled, but that didn't make any difference
Thanks @kverstae since browsers often use Alt modifier keys for other purposes.
I apologise for another massive PR! There are a few interesting changes here:
PServer
(file upload server) as it's replaced with HTTP API facilities now.Closes #234