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

17 breadcrumbs #54

Closed
wants to merge 93 commits into from
Closed

17 breadcrumbs #54

wants to merge 93 commits into from

Conversation

ri-pandey
Copy link
Contributor

@ri-pandey ri-pandey commented Aug 25, 2023

I also put in some loading indicators for smoother transitions between pages.

Worth pointing out that because multiple pages render the same components, and because we can't manually point routes to components, this has resulted in some of the pages being duplicated - like [datasetId].vue, which needs to be duplicated multiple times, since it can be reached from multiple URLs (/datasets/:id, rawdata/:id, dataproducts/:id). Although none of these pages have any business logic, so no actual 'code' is being duplicated here.

@ri-pandey ri-pandey marked this pull request as ready for review August 25, 2023 22:05
@deepakduggirala
Copy link
Contributor

@ri-pandey

  1. It seems like we're going to great lengths to adapt to the file-routing approach by creating multiple pages. Could you please investigate whether it's possible to directly add routes to the router object? Could you create a list of advantages and disadvantages for both approaches and present your reasoning for one of them.

  2. Would you be able to clarify the reasoning behind the creation of dataset and project stores?

  3. It would be helpful if you could include instructions for fellow developers regarding how to configure the breadcrumbs when adding new nested pages. Ideally, the changes required should be minimal and closely related to the page components they're working on.

Since this change affects the framework at large, rather than just the feature level, it's important to consider how easy this design will be for other developers to utilize. When others have to add a new page, which specific files will require modification? Is the process straightforward? A relevant example is the capability to set the page title. This can be done simply by including title: <page title> within the route section of the new page component. This adjustment doesn't require any modifications to other files.

  1. Page title is duplicated is raw data, data product, project, user, profile pages. Could you remove the page titles as they are already in the breadcrumbs?
image
  1. Because we used /datasets/[id] url to show all types of dataset pages, the sidebar items are not highlighted when a raw data or data product page is viewed. Thank you for fixing this by adding new pages per dataset type. However, when clicking on an associated dataset link when in a dataset page, the route rendered is /datasets/[id] which removes the highlight from the sidebar item. I think it is better to get rid of the /datasets/[id] url and use dataset type specific urls you have added.

  2. While viewing a dataset from the project page, the dataset type is not visible. In some applications, the names of raw data and derived data products might be the same. Consequently, it might not be immediately evident to users whether they're viewing raw data or a data product. One potential solution is to incorporate the dataset type within the breadcrumb list, addressing this concern.

Ex:
Projects > ILMN_2518_Jackson_DNAseq8_June2023 > Raw data: PCM230203 instead of Projects > ILMN_2518_Jackson_DNAseq8_June2023 > Dataset > PCM230203

image

@charlesbrandt

@ri-pandey
Copy link
Contributor Author

@deepakduggirala,

  1. Yes, I can investigate options to reduce page duplication.
  2. The project and dataset details are needed so we can show those in the breadcrumbs (project slug and dataset name). So far we have only been fetching the project/dataset in the Project/Dataset components. However, external components like LeaveBreadcrumbs can't access a component's local state. Therefore it's better to store any information that may be needed globally - like project/dataset - to a store. I also made sure that the Project/Dataset components doesn't re-fetch the project/dataset upon loading, since they would have already been fetched by the breadcrumbs component.
  3. Will do
  4. Noted
  5. Will do
  6. Each Dataset can render a list of datasets (source datasets and derived datasets). If a user is viewing a dataset under the Raw Data page, is it guaranteed that all the source/derived datasets for this dataset will also belong to the 'Raw Data' category? I assumed this wasn't necessary, hence in the current design, all source/derived datasets redirect to '/datasets/...'
  7. Yes, I can do that.

@ri-pandey ri-pandey force-pushed the 17-breadcrumbs branch 4 times, most recently from f583bd9 to d0cf7b5 Compare August 31, 2023 17:38
@deepakduggirala
Copy link
Contributor

Merged as a part of #65

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.

2 participants