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

Remove Breadcrumbs component #1105

Merged

Conversation

MarcelRobitaille
Copy link
Collaborator

  • Remove the troublesome Breadcrumbs component.
  • Show the "location" (recipe name, category name, etc) in an h2. Consistently show the "mode" where applicable (Viewing category, Viewing recipe, Editing recipe). Previously, "Edit" was shown when editing, but nothing was shown when viewing. This should be more clear and consistent.
  • Move all of the buttons to the right of the bar. Each page should have a primary button (Edit, Save, Search) and all the rest go into an overflow 3-dot menu

To-do:

  • Fix translations. "Save changes" was changed to "Save". "Edit recipe" was changed to "Edit". Some new strings were added: "Viewing recipe", "Editing recipe", "Creating new recipe"

Fixes #281
Fixes #896

Peek.2022-07-23.16-57.mp4

image

@github-actions
Copy link

github-actions bot commented Jul 23, 2022

Unit Test Results

     36 files       36 suites   10m 46s ⏱️
   403 tests    403 ✔️ 0 💤 0
4 836 runs  4 836 ✔️ 0 💤 0

Results for commit 2fa266f.

♻️ This comment has been updated with latest results.

@christianlupus
Copy link
Collaborator

Hey, @MarcelRobitaille. I just tried with the result a bit. It is working great and incredibly. There is no glitch and I have the impression of a very clean UI.

I just found one thing: If the display gets narrow enough, the left menu slides away. However, there is no longer a button to enlarge the side menu (see image):

grafik

@MarcelRobitaille
Copy link
Collaborator Author

Hi @christianlupus. I am glad you like it.

I noticed that button was missing as well. It's missing in master though (screenshot below is with master checked out). There is a space for the button, but no button. I thought it was unrelated to this. Do you know why this would be? Knowing how it was taken out / how it was implemented before would help me add it back here.
image

@christianlupus
Copy link
Collaborator

It is a standard component. See the docs at https://nextcloud-vue-components.netlify.app/#/Components/App%20containers?id=appnavigation.

grafik

@christianlupus christianlupus force-pushed the 896-primary-edit-button branch from 3c5df9a to f8b73b7 Compare July 24, 2022 12:12
@MarcelRobitaille
Copy link
Collaborator Author

I found the problem. The old button was actually still there, but underneath the main content. I made my window narrow and did $0.click() on that button, and the navigation was also under the main content. There seems to have been a z-index change that caused the AppNavigation and the associated toggle button to go underneath the main content.

christianlupus and others added 8 commits July 24, 2022 23:17
Signed-off-by: Marcel Robitaille <[email protected]>
Signed-off-by: Christian Wolf <[email protected]>
- Remove home button
- Consistently show the "mode" (editing recipe, viewing recipe, viewing
  category)
- Always show the "location" (recipe name, "All recipes", category name)

Signed-off-by: Marcel Robitaille <[email protected]>
Signed-off-by: Christian Wolf <[email protected]>
Signed-off-by: Marcel Robitaille <[email protected]>
Signed-off-by: Christian Wolf <[email protected]>
Makes the root template more concise and explicit and makes sure you get
the class name right

Signed-off-by: Marcel Robitaille <[email protected]>
Signed-off-by: Marcel Robitaille <[email protected]>
@MarcelRobitaille MarcelRobitaille force-pushed the 896-primary-edit-button branch from 1bedac8 to 2d547ad Compare July 25, 2022 03:17
Signed-off-by: Marcel Robitaille <[email protected]>
@MarcelRobitaille MarcelRobitaille marked this pull request as ready for review July 26, 2022 02:49
@christianlupus christianlupus added this to the Release 0.9.14 milestone Jul 26, 2022
Copy link
Collaborator

@christianlupus christianlupus left a comment

Choose a reason for hiding this comment

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

I have not worked my way through every single line but this seems working pretty well

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.

UI glitch with narrow devices Papercut: In top navigation: hand-cursor without a link
2 participants