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

Merging updates on angular 18 #299

Merged
merged 7 commits into from
Jul 20, 2024
Merged

Merging updates on angular 18 #299

merged 7 commits into from
Jul 20, 2024

Conversation

shb0527
Copy link
Collaborator

@shb0527 shb0527 commented Jul 15, 2024

  • Remembering registry & collection
  • Display ID showing errors if no input text

@cjmyers
Copy link
Contributor

cjmyers commented Jul 16, 2024

For the collections, you need to remember all the collections in the list, so you can back out of the collection selection. For example, if you select the iGEM collection, followed by the categories collection, then select a specific category. Then, when you import a component next, the collections menu needs to include all the collections selected into, so you can back out later.

@cjmyers
Copy link
Contributor

cjmyers commented Jul 16, 2024

Also, for the displayId, I can clear it. Without tab to the next line, and it clears the displayId with no warning.

@Drock54651
Copy link
Collaborator

It will also throw an error if the user goes from the synbiohub server to the empty option value in the Server dropdown.

@shb0527
Copy link
Collaborator Author

shb0527 commented Jul 17, 2024

For the collections, you need to remember all the collections in the list, so you can back out of the collection selection. For example, if you select the iGEM collection, followed by the categories collection, then select a specific category. Then, when you import a component next, the collections menu needs to include all the collections selected into, so you can back out later.

@cjmyers I changed the code to remember the history of the collection list so the user can see the list in the dropdown.

changed file is here -> 243255c

@shb0527
Copy link
Collaborator Author

shb0527 commented Jul 17, 2024

Collaborator

I fixed this error too.

@cjmyers
Copy link
Contributor

cjmyers commented Jul 17, 2024

The collection list seems to include all collections ever selected rather than just the latest ones. To see this issue, select the iGEM collection, then select categories, then go back to iGEM collection and import a component. Then select another component and look at the list of collections. Should only have Root and iGEM Collection. Similarly, if you go back to the Root collection and select the Cello collection and select a component from there. It will still have both the iGEM and Cello collection next time you import a component.

@cjmyers
Copy link
Contributor

cjmyers commented Jul 17, 2024

As for the displayId, I'm able to clear displayIds. However, you do get a warning on the upper right part of the screen, which is good. Not sure if this can be improved more at this point.

@Drock54651
Copy link
Collaborator

Perhaps check if the user enters an empty input and just reject it?

@shb0527
Copy link
Collaborator Author

shb0527 commented Jul 18, 2024

The collection list seems to include all collections ever selected rather than just the latest ones. To see this issue, select the iGEM collection, then select categories, then go back to iGEM collection and import a component. Then select another component and look at the list of collections. Should only have Root and iGEM Collection. Similarly, if you go back to the Root collection and select the Cello collection and select a component from there. It will still have both the iGEM and Cello collection next time you import a component.

The collection list seems to include all collections ever selected rather than just the latest ones. To see this issue, select the iGEM collection, then select categories, then go back to iGEM collection and import a component. Then select another component and look at the list of collections. Should only have Root and iGEM Collection. Similarly, if you go back to the Root collection and select the Cello collection and select a component from there. It will still have both the iGEM and Cello collection next time you import a component.

@cjmyers I think now I finally understood what has to be done, and it seems to work properly.

changed code is here -> 5eb46b3

@shb0527
Copy link
Collaborator Author

shb0527 commented Jul 18, 2024

Perhaps check if the user enters an empty input and just reject it?

Perhaps check if the user enters an empty input and just reject it?

Oh it shows an error line, but it still allows the empty input.

@shb0527
Copy link
Collaborator Author

shb0527 commented Jul 18, 2024

As for the displayId, I'm able to clear displayIds. However, you do get a warning on the upper right part of the screen, which is good. Not sure if this can be improved more at this point.

Oh I was thinking of another option to just prohibit the user to change the text. So making the displayID to be fixed as it is randomly generated, and if it is fixed, the red warning sign would not be shown on the upper right of the screen.

@cjmyers
Copy link
Contributor

cjmyers commented Jul 18, 2024

No, users need to be able to change the displayId, just not to an empty or invalid string (i.e., must be alphanumeric or underscore and not starting with a number).

@cjmyers
Copy link
Contributor

cjmyers commented Jul 18, 2024

Has your update to the collection memory been checked in? It is still not working for me. If you have a list of collections in the menu, and you go to an earlier item in the list, it should remove the later items in the list.

@shb0527
Copy link
Collaborator Author

shb0527 commented Jul 18, 2024

Has your update to the collection memory been checked in? It is still not working for me. If you have a list of collections in the menu, and you go to an earlier item in the list, it should remove the later items in the list.

I just added some code and it seems to work now. >> 09d8eb3

@cjmyers
Copy link
Contributor

cjmyers commented Jul 19, 2024

Currently it is not going into the selected collection when you open the import window again.

Also, I can still erase a display id completely, would be nice to just revert to previous value if someone tries to clear it.

@shb0527
Copy link
Collaborator Author

shb0527 commented Jul 19, 2024

Currently it is not going into the selected collection when you open the import window again.

Also, I can still erase a display id completely, would be nice to just revert to previous value if someone tries to clear it.

Now empty input is not allowed as well.

@cjmyers
Copy link
Contributor

cjmyers commented Jul 20, 2024

closes #223 closes #111

@cjmyers
Copy link
Contributor

cjmyers commented Jul 20, 2024

This works now. Merging.

@cjmyers cjmyers merged commit b869334 into final Jul 20, 2024
2 of 3 checks passed
@cjmyers cjmyers deleted the newChanges_angular18 branch July 20, 2024 00:52
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.

3 participants