Skip to content

Breadcrumb Navigation #1902

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

Merged
merged 181 commits into from
Oct 28, 2020
Merged

Conversation

odeimaiz
Copy link
Member

@odeimaiz odeimaiz commented Oct 26, 2020

What do these changes do?

This PR brings nicer a breadcrumb navigation plus some improvements in the slideshow viewer.

Related issue number

related to ITISFoundation/osparc-issues#327
closes ITISFoundation/osparc-issues#359

  • Workbench mode:
    image

  • Guided mode:
    image

Bonus:

  • Some Grouping features were brought back
  • New TI Treatment Planning logo
    image

How to test

Checklist

  • Did you change any service's API? Then make sure to bundle document and upgrade version (make openapi-specs, git commit ... and then make version-*)
  • Unit tests for the changes exist
  • Runs in the swarm
  • Documentation reflects the changes
  • New module? Add your github username to .github/CODEOWNERS

Copy link
Contributor

@ignapas ignapas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's amazing how you managed to do this! Very nice... although the amount of code needed (svg library + your own) to create the breadcrumb splitter, I think it's too much. I wouldn't do this kind of personalised widgets too often.
Some little things:

  • A controlled breadcrumb widget. Instead of having the logic in NavigationBar, having a separate Breadcrumb widget that takes a path of node ids, and renders itself as a series of ToggleButtons and separators, or even better, as a series of extended ToggleButtons that incorporate their separators when appropriate, and manage their state changes on hover, clicking, etc.
  • On hover, the separator is not highlighted and looks a bit weird.
    image
  • I don't know if related to this PR, but grouping nodes no longer works. Links created inside the group are not saved, and the edge stays dashed. I cannot ungroup them either..
    groupingfail
    image
  • I see some hardcoded widths and heights

@odeimaiz
Copy link
Member Author

odeimaiz commented Oct 27, 2020

@GitHK @ignapas To be tackled in this PR:

  • If I add too many slides, the layout is broken and the next and prev buttons disappear; also the breadcrumb bar on top is not fully visible.
  • A controlled breadcrumb widget
  • Remove hardcoded widths and heights
  • On hover, the separator is not highlighted and looks a bit weird

@odeimaiz odeimaiz requested review from ignapas and GitHK October 27, 2020 13:52
Copy link
Contributor

@ignapas ignapas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome!!
Sorry, there is something I didn't see before... not so important... there is a problem when you are inside some group views, the breadcrumb disappears and the previous version shows up
groupsandbread

@odeimaiz odeimaiz merged commit 66bdc07 into ITISFoundation:master Oct 28, 2020
@odeimaiz odeimaiz deleted the feature/breadcrumbs branch November 26, 2020 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:frontend issue affecting the front-end (area group)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FilePicker in Guided mode
6 participants