Skip to content

Conversation

@abigailalexander
Copy link
Collaborator

I implemented these changes to come up with a solution to issues I faced when creating a client application that opens and displays files.

Suggested changes:

  • Allow new files to be opened at a new URL if wanted. Currently new files are opened at the same address, but a new entry is added to the history. This allows you to traverse file history but not customise URL based on the currently displayed file. Now you can optionally pass a pathname to open your new screen at. It might be worth also adding this to the removePage option, I'd appreciate any opinions?
  • Export the executeAction hook so that non cs-web-lib components can trigger open/close page/tab events. Currently, actions such as opening a screen can only be called from inside an existing screen (either a .bob file or a json screen). This seems quite limited if we want to display dynamic .bob files as part of a larger non cs-web-lib application

To test these changes, you can clone https://github.com/DiamondLightSource/daedalus and use a local build of cs-web-lib with these changes. You should be able to see with these changes how .bob files can be opened through the .bob file screens themselves, but also through a menubar and breadcrumbs trail, with the URL changing when a new file is opened.

I think overall the structure of adding tabs/pages needs an overhaul, but that would be better done at a separate time once we're sure what approach we want to take with it. For now, these changes expand the use of cs-web-lib while not changing the behaviour.

@rjwills28
Copy link
Collaborator

Thanks for this PR and the useful example on how to test it. It's nice that the URL changes as you open pages. I just have a few quick questions:

  1. If I go straight to the URL http://localhost:5173/BLTEST for example, I end up in a state where I can't do/see anything. The page is empty and there is no menu on the left to select a page. If I try to reselect the beamline it still doesn't do anything. The only way I could get back to anything I could interact with was to remove the "BLTEST" and go to http://localhost:5173/. Is that expected?
  2. In terms of the 'removePage' I see that you can remove the entire page and the URL stays the same. Not sure what the desired behaviour would be in this example though. Should it go back to the previous path or the root path - not sure. I think the only way to remove the page is to enable the showCloseButton={true} and close from there so maybe not relevant in this case?

@abigailalexander
Copy link
Collaborator Author

Hi Becky, thanks for looking over the PR. Yes, that is expected (although not desired). It's a bug with how I first load the files that needs fixing.

You might be able to remove a page by triggering an action through a button click, but overall it does seem like the only way to remove a page is via that optional close button. I suppose the desired behaviour of where the path returns to would be up to the client application to decide, and whether to use the feature or not if it is included. It does seem like a slightly different use case to the existing one.

@rjwills28
Copy link
Collaborator

Hi Abii, thanks for the details. OK, let's leave the 'removePage' for the moment. Also just to mention that the showCloseButton functionality is in cs-web-lib but the image of the 'cross' to show in the corner of display to close it is actually still in cs-web-proto... That should be fixed at some point as a missing image symbol gets shown at the moment.

Otherwise I'm happy with this PR - was there something you wanted to fix or was that for a later date?

@abigailalexander
Copy link
Collaborator Author

Good spot, I hadn't noticed the missing image symbol.

There wasn't anything else to fix, I think it's fine as it is now. Thanks :)

@abigailalexander abigailalexander merged commit 8d9e535 into master Feb 24, 2025
2 checks passed
@abigailalexander abigailalexander deleted the export-file-context-actions branch February 24, 2025 10:30
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