-
Notifications
You must be signed in to change notification settings - Fork 63
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
Improve adding assemblies internally, and use the assembly displayName in more places in the UI #3180
Conversation
can you add context about this PR e.g. what made these come up? they are probably good changes but having background is helpful :) |
Some assemblies in Apollo are currently handled as session assemblies, so I want users to be able to select them in e.g. an LGV. Also, those assemblies have a session.addAssembly(assemblyConfig)
const assembly = await assemblyManager.waitForAssembly(assemblyConfig.name)
// do stuff with assembly That didn't work before, but with these changes it does. |
I guess that sounds ok. It is a little bit hard to decide the difference between name and displayName, because to me name is still user-readable in the form of something like hg19 where I would say displayName is "Homo sapiences (hg19)" to make it abundantly clear. In some cases where name is less familiar, then displayName is used (long assembly selectors), but I still thought name is a useful short form of it. It is called a "name" and not an "id", even though it's treated as a unique id If this is changed to displayname, it is probably ok, but it will look more verbose most likely in some cases. And there are many places that do I think suppose assembly "name" is displayable...example is the title in the view container |
I would consider that assembly name may be useful to display in places in our UI still. the behavior could be maybe kind of like username and your real name being displayed on a website... |
This PR falls back to the name when there is no displayName, so for many cases it won't change anything. But if a user configures a displayName, it seems like that's what they would expect to see in most cases. My concern is that "name" is the |
alrighty, we'll go ahead with merge them |
This has a few different changes related to assemblies:
assemblyNames
, this now includes the session assemblies' names as well. This has the effect of being able to choose session assemblies from the assembly selector in e.g. the LGV setup screen.displayName
instead ofname
in more user-facing places.waitForAssembly
on the assembly manager. Before, if you added an assembly and then immediately calledwaitForAssembly
, it wouldn't work because the reaction that adds the new assembly hadn't fired yet. Now, if it can't find the assembly it waits for a second and tries again to give the reaction time to run.