Skip to content
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

Open
wants to merge 40 commits into
base: develop
Choose a base branch
from
Open

Viewer redesign #510

wants to merge 40 commits into from

Conversation

MaybeJustJames
Copy link
Collaborator

@MaybeJustJames MaybeJustJames commented Mar 9, 2022

I apologise for another massive PR! There are a few interesting changes here:

  1. The "Gene" and "Compare" pages have been merged. They do NOT display anything yet (except for "Empty" or project UUID + dataset index).
  2. A basic login modal is displayed when the application loads
  3. After login, a button with the username is displayed in the top-right.
  4. Adds a bunch of tests
  5. Replaces Semantic-UI-CSS with maintained and compatible Formatic-UI-CSS
  6. Remove the PServer (file upload server) as it's replaced with HTTP API facilities now.

Closes #234

* 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
Copy link
Collaborator

@kverstae kverstae left a 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

client/src/components/AppHeader.tsx Show resolved Hide resolved
client/src/components/Main.tsx Outdated Show resolved Hide resolved
client/src/components/Search/FeatureSearch.tsx Outdated Show resolved Hide resolved
client/src/components/Viewer/ViewerWrapper.tsx Outdated Show resolved Hide resolved
@@ -30,7 +30,7 @@ const ProjectView: React.FC<ProjectViewProps> = (props) => {
const [displayUpload, setDisplayUpload] = useState(false);
Copy link
Collaborator

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.

Copy link
Collaborator Author

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?

Copy link
Collaborator

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):

  1. create project 1 (OK) -> add dataset 1 (OK) -> add dataset 2 (no popup)
  2. create project 1 (OK) -> add dataset 1 (OK) -> create project 2 (OK) -> add dataset to project 2 (no popup)

}
})}

{state.remove ? (
Copy link
Collaborator

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.

Copy link
Collaborator Author

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?

Copy link
Contributor

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.

Copy link
Collaborator

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

Copy link
Collaborator Author

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)?

Copy link
Collaborator

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

client/src/components/pages/Viewer.tsx Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants