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

[WIP] feat(NcAppNavigation): add top-bar slot to save space for toggle button #4639

Closed
wants to merge 1 commit into from

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Oct 12, 2023

☑️ Resolves

The toggle button outside the navigation doesn't always look good. The problem is that having this button inside the navigation may not fit any layout and requires saving space for it on every app's side.

The idea is to have an additional top-bar slot to allow some content in a special place at the top of navigation that reserves space for the toggle button.

This slot can be optional, keeping the current behavior as a default behavior to not make a breaking change here.

Usage example:

<NcAppNavigation>
  <template #top-bar>
    <SomeSearch />
    <!-- or -->
    <SomePrimaryButton />
  </template>
  
  <!-- Navigation items -->
</NcAppNavigation>

🖼️ Screenshots

Default With optional top-bar slot
image image

PS: Talk could be not the best example because of many items in the top already.

🚧 Tasks

  • ...

Alternative solution

Instead of providing a slot to render top bar in a specific position with a toggle in a specific position, we can also provide a possibility to just render NcAppNavigaroinToggle button wherever application wants directly. Aka

<NcAppNavigation>
  <div class="top">
    <NcAppNavigationToggle /> <!-- Just put the button where you want -->
    <!-- some other top content -->
  </div>
  <!-- Some navigation elements -->
</NcAppNavigation>

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

This looks like a good idea in general, however I guess this will not fix our problem in the files app as it will look weird. See
image
WDYT @nextcloud-libraries/designers

So maybe the toggle should be positioned by default on the right and not on the left of the first item? Also the sidebar should probably go full screen on mobile then?

@nimishavijay
Copy link

This issue has come up many times before too. @ShGKme's proposal is great because the position of your cursor doesn't have to move to open and close the navigation.

We could think of standardizing the structure for the navigation such that there is always either search or a button as the first item. This already exists in Mail, Calendar, Contacts and Talk. We would have to change it for Files and Photos. This kind of layout is also consistent with competitors like Gdrive and OneDrive. What do you think?

image
image

@ShGKme
Copy link
Contributor Author

ShGKme commented Oct 13, 2023

This kind of layout is also consistent with competitors like Gdrive and OneDrive. What do you think?

It seems that they don't have sidebar toggle button at all...

@nimishavijay
Copy link

It seems that they don't have sidebar toggle button at all...

Yes, the navigation is always showing, I meant that the "New item" button is always at the top of the navigation :)

@jancborchardt
Copy link
Contributor

We had this discussion at length with many proposals before and there doesn't seem to be a really good option. Ref previous threads:

The current one makes sense placement-wise, and it is also the placement we use for the right sidebar.

While this one here has the benefit of the mouse not needing to move when you toggle it, this seems only beneficial when you toggle the nav on and off immediately (not a very common case?), and sacrificing a more logical placement for it.

The problem is that having this button […] requires saving space for it on every app's side.

Where is the toggle when the navigation is closed?

This slot can be optional, keeping the current behavior as a default behavior to not make a breaking change here.

If we would go for something we should force it as a standard, otherwise the experience will be off.

Talk could be not the best example because of many items in the top already.

It seems we are mainly trading "saving space in every apps content" with "saving space in every apps navigation". ;)


All in all, considering all the solutions we already discussed in the past and the difficulty to align this, I am very hesitant to have us make any change here. The current way works quite well by now, is established, and the placement itself makes sense.

@dartcafe
Copy link
Contributor

The current one makes sense placement-wise, and it is also the placement we use for the right sidebar.

At the risk of being annoying, but I still have another opinion about that. The position is annoying (I think I mentioned it one or two times before 😉), because it overlaps the app-content region and every app developer has to fiddle around this position and find a solution for himself.

This forces apps a whole column or a (sticky) header row, to avoid conflicts with the app's content, especially when it comes to scrolling. This leads to an inconsitent appearence of all apps using the navigation bar, depending of the solution the app developer choosed. Some old impressions are found here

From an app devolpers view I would prefer the toggle as integrated component of the navigation bar. This means it is outside the app container does not interfer with the app.

From a user's view I would prefer a consitent presentation of central actions. The Navigation bar is the same in every app, which uses it. So should the toggle be also.

Therfore I would like to throw my prior proposal back into the discussion.

@ShGKme
Copy link
Contributor Author

ShGKme commented Oct 18, 2023

The current way works quite well by now

Except small screens. This toggle button doesn't allow using all the screen width for the sidebar.

Navigation Sidebar
image image

@dartcafe
Copy link
Contributor

As you mention small screens:

talk mail calendar polls
grafik grafik grafik grafik

@ShGKme
Copy link
Contributor Author

ShGKme commented Oct 18, 2023

Working on an alternative solution to only fix issues with a small and middle screen without any fundamental changes

@susnux susnux added this to the 8.1.0 milestone Nov 8, 2023
@AndyScherzinger AndyScherzinger modified the milestones: 8.1.0, 8.1.1 Nov 14, 2023
@susnux susnux modified the milestones: 8.1.1, 8.2.0 Nov 14, 2023
@ShGKme
Copy link
Contributor Author

ShGKme commented Nov 17, 2023

Closed in flavor of #4767

@ShGKme ShGKme closed this Nov 17, 2023
@ShGKme ShGKme removed this from the 8.2.0 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design, UX, interface and interaction design enhancement New feature or request feature: app-navigation Related to the app-navigation component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants