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

Implement highlightable description #380

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/components/HeaderForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import ExampleSelectButtons from "./ExampleSelectButtons";
import RegionInput from "./RegionInput";
import TrackPicker from "./TrackPicker";
import BedFileDropdown from "./BedFileDropdown";
import FormHelperText from "@mui/material/FormHelperText";
import { parseRegion, stringifyRegion, isEmpty, readsExist } from "../common.mjs";


Expand Down Expand Up @@ -38,6 +39,9 @@ const CLEAR_STATE = {
// File: The file name actually used (or undefined)
bedSelect: "none",

// Description for the selected region, is not displayed when empty
desc: "",

// This tracks several arrays of BED region data, stored by data type, with
// one entry in each array per region.
regionInfo: {},
Expand Down Expand Up @@ -519,7 +523,10 @@ class HeaderForm extends Component {
// Update current track if the new tracks are valid
// Otherwise check if the current bed file is a url, and if tracks can be fetched from said url
// Tracks remain unchanged if neither condition is met
handleRegionChange = async (value) => {
handleRegionChange = async (value, desc) => {
// Update region description
this.setState({ desc: desc });

// After user selects a region name or coordinates,
// update path, region, and associated tracks(if applicable)

Expand Down Expand Up @@ -761,6 +768,7 @@ toggleSimplify = () => {
const customFilesFlag = this.state.dataType === dataTypes.CUSTOM_FILES;
const examplesFlag = this.state.dataType === dataTypes.EXAMPLES;
const viewTargetHasChange = !viewTargetsEqual(this.getNextViewTarget(), this.props.getCurrentViewTarget());
const displayDescription = this.state.desc;


console.log(
Expand Down Expand Up @@ -883,6 +891,12 @@ toggleSimplify = () => {
!customFilesFlag && DataPositionFormRowComponent
)}
</Row>
{displayDescription ?
<div style={{marginTop: "10px"}}>
<FormHelperText> {"Region Description: "} </FormHelperText>
<FormHelperText style={{fontWeight: "bold"}}>{this.state.desc}</FormHelperText>
</div>
: null }
</Col>
</Row>
</Container>
Expand Down
29 changes: 18 additions & 11 deletions src/components/RegionInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,23 @@ export const RegionInput = ({
// Generate autocomplete options for regions from regionInfo
// Add : to pathNames
console.log("rendering with pathnames: ", pathNames);
const pathNamesColon = pathNames.map((name) => name + ":");
const pathNamesColon = pathNames.map((name) => {
return {label: name + ":", value: name + ":"};
});
const pathsWithRegion = [];

const regionToDesc = new Map();

if (regionInfo && !isEmpty(regionInfo)) {
// Stitch path name + region start and end
for (const [index, path] of regionInfo["chr"].entries()) {
const pathWithRegion = path + ":" + regionInfo.start[index] + "-" + regionInfo.end[index];
pathsWithRegion.push(pathWithRegion);
const pathWithRegion = path + ":" + regionInfo.start[index] + "-" + regionInfo.end[index]
pathsWithRegion.push({
label: pathWithRegion + " " + regionInfo.desc[index],
value: pathWithRegion
});
regionToDesc.set(pathWithRegion, regionInfo.desc[index]);
}

// Add descriptions
pathsWithRegion.push(...regionInfo["desc"]);

}

// Autocomplete selectable options
Expand All @@ -52,14 +53,20 @@ export const RegionInput = ({
<Autocomplete
disablePortal
freeSolo // Allows custom input outside of the options
getOptionLabel={(option) => option.title || option.toString()}
getOptionLabel={(option) => option.label || option.toString()}
value={region}
inputValue={region}
data-testid="autocomplete"
id="regionInput"

onInputChange={(event, newInputValue) => {
handleRegionChange(newInputValue);
onInputChange={(event, newInput) => {
let regionValue = newInput;
const regionObject = displayRegions.find((option) => option.label === newInput);
// If input is selected from one of the options
if (regionObject) {
regionValue = regionObject.value
}
handleRegionChange(regionValue, regionToDesc.get(regionValue));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like the regionToDesc lookup might really want to be in handleRegionChange? So it doesn't need a new argument? But we kind of maintain the whole database of regions in RegionInput and it would be odd to do it twice.

I think Shreya is working on the left/right button jump-to-next-region thing in #354 though and that requires having more of an understanding of the region database in the header form, so I think this sort of thing is moving out of RegionInput in the medium term.

}}

options={displayRegions}
Expand Down Expand Up @@ -101,4 +108,4 @@ RegionInput.propTypes = {
}),
};

export default RegionInput;
export default RegionInput;
4 changes: 2 additions & 2 deletions src/components/RegionInput.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,5 @@ test("it calls handleRegionChange when region is changed with new region", async
await userEvent.clear(input);
await userEvent.type(input, NEW_REGION);

expect(handleRegionChangeMock).toHaveBeenLastCalledWith(NEW_REGION);
});
expect(handleRegionChangeMock).toHaveBeenLastCalledWith(NEW_REGION, undefined);
});
10 changes: 4 additions & 6 deletions src/end-to-end.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import App from "./App";

const getRegionInput = () => {
// Helper function to select the Region input box
return screen.getByTestId("autocomplete").querySelector("input");
return screen.getByRole("combobox", { name: /Region/i });
};
// This holds the running server for the duration of each test.
let serverState = undefined;
Expand Down Expand Up @@ -216,10 +216,8 @@ describe("When we wait for it to load", () => {
await act(async () => {
userEvent.click(getRegionInput());
});

// Make sure that option in RegionInput dropdown (17_1_100) is visible
const text = screen.getAllByText("17_1_100");
expect(text[0]).toBeInTheDocument();
expect(screen.getByText("17:1-100 17_1_100")).toBeInTheDocument();
});
it("the region options in autocomplete are cleared after selecting new data", async () => {
// Input data dropdown
Expand All @@ -232,7 +230,7 @@ describe("When we wait for it to load", () => {
userEvent.click(getRegionInput());
});
// Make sure that old option in RegionInput dropdown (17_...) is not visible
expect(screen.queryByText("17_1_100")).not.toBeInTheDocument();
expect(screen.queryByText("1-100 17_1_100")).not.toBeInTheDocument();
await act(async () => {
userEvent.click(regionInput);
});
Expand Down Expand Up @@ -535,4 +533,4 @@ it("can accept uploaded files", async () => {

console.log("Test over");
}, 50000); // We need to allow a long time for the slow vg test machines.
// TODO: Is this slow because of unnecessary re-renders caused by the new color schemes taking effect and being rendered with the old data, before the new data downloads?
// TODO: Is this slow because of unnecessary re-renders caused by the new color schemes taking effect and being rendered with the old data, before the new data downloads?
Loading