SynBioDex / SBOLCanvas

Apache License 2.0
11 stars 6 forks source link

Merging updates on angular 18 #299

Closed shb0527 closed 1 month ago

shb0527 commented 1 month ago
cjmyers commented 1 month ago

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 commented 1 month ago

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

Drock54651 commented 1 month ago

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

shb0527 commented 1 month ago

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 -> https://github.com/SynBioDex/SBOLCanvas/pull/299/commits/243255c34f5dc0dc451138dc720e2cab90c21101

shb0527 commented 1 month ago

Collaborator

I fixed this error too.

cjmyers commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

shb0527 commented 1 month ago

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 -> https://github.com/SynBioDex/SBOLCanvas/pull/299/commits/5eb46b39984b2dd1cf93ff5636ad40fb18e80a81

shb0527 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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. >> https://github.com/SynBioDex/SBOLCanvas/pull/299/commits/09d8eb3b913f61dc51fb0e7380e435226792397e

cjmyers commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

closes #223 closes #111

cjmyers commented 1 month ago

This works now. Merging.