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.

Sign up for GitHub

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

Jump to bottom

Improve navigation sub-menu and tabs effects #2971

Merged
merged 11 commits into from
Jul 23, 2020
Merged

Improve navigation sub-menu and tabs effects #2971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
863cbc3
Improve nav border and colors on active and non-active
kimsible Jul 14, 2020
9eac1ac
Remove margin-top effect on active nav
kimsible Jul 15, 2020
dbe5937
Use opacity / bold instead of color change on nav
kimsible Jul 15, 2020
3161322
Remove dropdown items label in sub-menu and add a class active
kimsible Jul 17, 2020
08d30df
Position sub-menu to fixed
kimsible Jul 17, 2020
8440e87
Autoclose dropdown sub-menu only on outside click
kimsible Jul 17, 2020
1959ff3
Remove open dropdown on hover in sub-menu
kimsible Jul 17, 2020
9b775ec
Show reusable h1 for dropdown item in sub-menu
kimsible Jul 17, 2020
94b279e
Put reusable sub-menu h1 styles to mixins
kimsible Jul 17, 2020
4159931
Add icons to sub-menu dropdown-item h1
kimsible Jul 17, 2020
d6e5182
Make all the sub-menu accessible with focus
kimsible Jul 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions client/src/app/+admin/admin.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
@import '_variables';
@import '_mixins';

my-top-menu-dropdown {
flex-grow: 1;
}

@include sub-menu-h1;
5 changes: 5 additions & 0 deletions client/src/app/+admin/follows/followers-list/followers-list.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
<h1>
<my-global-icon iconName="follower" aria-hidden="true"></my-global-icon>
<ng-container i18n>Instances following you</ng-container>
</h1>

<p-table
[value]="followers" [lazy]="true" [paginator]="totalRecords > 0" [totalRecords]="totalRecords" [rows]="rowsPerPage" [rowsPerPageOptions]="rowsPerPageOptions"
[sortField]="sort.field" [sortOrder]="sort.order" (onLazyLoad)="loadLazy($event)" (onPage)="onPage($event)"
Expand Down
5 changes: 5 additions & 0 deletions client/src/app/+admin/follows/following-list/following-list.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
<h1>
<my-global-icon iconName="following" aria-hidden="true"></my-global-icon>
<ng-container i18n>Instances you follow</ng-container>
</h1>

<p-table
[value]="following" [lazy]="true" [paginator]="totalRecords > 0" [totalRecords]="totalRecords" [rows]="rowsPerPage" [rowsPerPageOptions]="rowsPerPageOptions"
[sortField]="sort.field" [sortOrder]="sort.order" (onLazyLoad)="loadLazy($event)" (onPage)="onPage($event)"
Expand Down
5 changes: 5 additions & 0 deletions client/src/app/+admin/follows/video-redundancies-list/video-redundancies-list.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
<h1>
<my-global-icon iconName="videos" aria-hidden="true"></my-global-icon>
<ng-container i18n>Videos redundancies</ng-container>
</h1>

<div class="admin-sub-header">
<div class="select-filter-block">
<label for="displayType" i18n>Display</label>
Expand Down
5 changes: 5 additions & 0 deletions client/src/app/+admin/moderation/abuse-list/abuse-list.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
<h1>
<my-global-icon iconName="flag" aria-hidden="true"></my-global-icon>
<ng-container i18n>Reports</ng-container>
</h1>

<p-table
[value]="abuses" [lazy]="true" [paginator]="totalRecords > 0" [totalRecords]="totalRecords" [rows]="rowsPerPage" [rowsPerPageOptions]="rowsPerPageOptions"
[sortField]="sort.field" [sortOrder]="sort.order" (onLazyLoad)="loadLazy($event)" dataKey="id" [resizableColumns]="true" [lazyLoadOnInit]="false"
Expand Down
5 changes: 5 additions & 0 deletions client/src/app/+admin/moderation/video-block-list/video-block-list.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
<h1>
<my-global-icon iconName="cross" aria-hidden="true"></my-global-icon>
<ng-container i18n>Video blocks</ng-container>
</h1>

<p-table
[value]="blocklist" [lazy]="true" [paginator]="totalRecords > 0" [totalRecords]="totalRecords" [rows]="rowsPerPage" [rowsPerPageOptions]="rowsPerPageOptions"
[sortField]="sort.field" [sortOrder]="sort.order" (onLazyLoad)="loadLazy($event)" dataKey="id"
Expand Down
2 changes: 0 additions & 2 deletions client/src/app/+admin/plugins/plugins.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
<div class="admin-sub-header">
<h1 i18n class="form-sub-title">Plugins/Themes</h1>

<div class="admin-sub-nav">
<a i18n routerLink="list-installed" routerLinkActive="active">Installed</a>

Expand Down
5 changes: 0 additions & 5 deletions client/src/app/+admin/plugins/plugins.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
@import '_variables';
@import '_mixins';

.form-sub-title {
flex-grow: 0;
margin-right: 30px;
}

@media screen and (max-width: $small-view) {
::ng-deep .plugins .plugin .first-row {
flex-wrap: wrap;
Expand Down
2 changes: 0 additions & 2 deletions client/src/app/+admin/system/system.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
<div class="admin-sub-header">
<h1 i18n class="form-sub-title">System</h1>

<div class="admin-sub-nav">
<a *ngIf="hasJobsRight()" i18n routerLink="jobs" routerLinkActive="active">Jobs</a>

Expand Down
6 changes: 5 additions & 1 deletion ...t/src/app/+my-account/+my-account-video-channels/my-account-video-channels.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
<h1 class="sr-only" i18n>My channels</h1>
<h1>
<my-global-icon iconName="channel" aria-hidden="true"></my-global-icon>
<ng-container i18n>My channels</ng-container>
</h1>

<div class="video-channels-header">
<a class="create-button" routerLink="create">
<my-global-icon iconName="add" aria-hidden="true"></my-global-icon>
Expand Down
6 changes: 5 additions & 1 deletion client/src/app/+my-account/my-account-history/my-account-history.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
<h1 class="sr-only" i18n>History</h1>
<h1>
<my-global-icon iconName="history" aria-hidden="true"></my-global-icon>
<ng-container i18n>My history</ng-container>
</h1>

<div class="top-buttons">
<div class="history-switch">
<p-inputSwitch [(ngModel)]="videosHistoryEnabled" (ngModelChange)="onVideosHistoryChange()"></p-inputSwitch>
Expand Down
6 changes: 5 additions & 1 deletion client/src/app/+my-account/my-account-ownership/my-account-ownership.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
<h1 class="sr-only" i18n>Ownership changes</h1>
<h1>
<my-global-icon iconName="download" aria-hidden="true"></my-global-icon>
<ng-container i18n>My ownership changes</ng-container>
</h1>

<p-table
[value]="videoChangeOwnerships"
[lazy]="true"
Expand Down
6 changes: 5 additions & 1 deletion client/src/app/+my-account/my-account-subscriptions/my-account-subscriptions.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
<h1 class="sr-only" i18n>Subscriptions</h1>
<h1>
<my-global-icon iconName="subscriptions" aria-hidden="true"></my-global-icon>
<ng-container i18n>My subscriptions</ng-container>
</h1>

<div class="no-results" i18n *ngIf="pagination.totalItems === 0">You don't have any subscriptions yet.</div>

<div class="video-channels" myInfiniteScroller [autoInit]="true" (nearOfBottom)="onNearOfBottom()" [dataObservable]="onDataSubject.asObservable()">
Expand Down
6 changes: 5 additions & 1 deletion client/src/app/+my-account/my-account-video-imports/my-account-video-imports.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
<h1 class="sr-only" i18n>Imports</h1>
<h1>
<my-global-icon iconName="cloud-download" aria-hidden="true"></my-global-icon>
<ng-container i18n>My imports</ng-container>
</h1>

<p-table
[value]="videoImports" [lazy]="true" [paginator]="totalRecords > 0" [totalRecords]="totalRecords" [rows]="rowsPerPage" [rowsPerPageOptions]="rowsPerPageOptions"
[sortField]="sort.field" [sortOrder]="sort.order" (onLazyLoad)="loadLazy($event)" dataKey="id"
Expand Down
8 changes: 6 additions & 2 deletions .../src/app/+my-account/my-account-video-playlists/my-account-video-playlists.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
<div class="video-playlists-header">
<h1 i18n>Playlists <span class="badge badge-secondary">{{ pagination.totalItems }}</span></h1>
<h1>
<my-global-icon iconName="playlists" aria-hidden="true"></my-global-icon>
<ng-container i18n>My playlists</ng-container> <span class="badge badge-secondary">{{ pagination.totalItems }}</span>
</h1>


<div class="video-playlists-header">
<input type="text" placeholder="Search your playlists" i18n-placeholder [(ngModel)]="videoPlaylistsSearch" (ngModelChange)="onVideoPlaylistSearchChanged()" />

<a class="create-button" routerLink="create">
Expand Down
4 changes: 0 additions & 4 deletions .../src/app/+my-account/my-account-video-playlists/my-account-video-playlists.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@
input[type=text] {
@include peertube-input-text(300px);
}

h1 {
font-size: 1.5rem;
}
}

@media screen and (max-width: $small-view) {
Expand Down
9 changes: 5 additions & 4 deletions client/src/app/+my-account/my-account-videos/my-account-videos.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
<div class="videos-header">
<h1 i18n>Videos <span class="badge badge-secondary">{{ pagination.totalItems }}</span></h1>
<h1>
<my-global-icon iconName="videos" aria-hidden="true"></my-global-icon>
<ng-container i18n>My videos</ng-container><span class="badge badge-secondary"> {{ pagination.totalItems }}</span>
</h1>

<div class="videos-header">
<input type="text" placeholder="Search your videos" i18n-placeholder [(ngModel)]="videosSearch" (ngModelChange)="onVideosSearchChanged()" />

<div class="fake-element"></div>
</div>

<my-videos-selection
Expand Down
6 changes: 0 additions & 6 deletions client/src/app/+my-account/my-account-videos/my-account-videos.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@
justify-content: space-between;
margin: 20px 0 50px;

h1,
.fake-element {
flex: 1;
font-size: 1.5rem;
}

input[type=text] {
@include peertube-input-text(300px);
}
Expand Down
5 changes: 5 additions & 0 deletions client/src/app/+my-account/my-account.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
@import '_variables';
@import '_mixins';

.row {
flex-direction: column;
width: 100%;

& > my-top-menu-dropdown:nth-child(1) {
flex-grow: 1;
}

@include sub-menu-h1;
}
19 changes: 14 additions & 5 deletions client/src/app/+videos/+video-edit/video-add.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,25 @@ $nav-link-height: 40px;
padding: 0 30px !important;
font-size: 15px;

border: $border-width $border-type transparent;

span {
border-bottom: 2px solid transparent;
}

&.active {
border: $border-width $border-type $border-color;
border-bottom: none;
border-color: $border-color;
border-bottom-color: transparent;
background-color: pvar(--submenuColor) !important;

span {
border-bottom: 2px solid pvar(--mainColor);
font-weight: $font-bold;
border-bottom-color: pvar(--mainColor);
}
}

&:hover:not(.active) {
border-color: transparent;
}
}
}

Expand Down Expand Up @@ -71,7 +80,7 @@ $nav-link-height: 40px;

/* Hide active tab style to not have a moving tab effect */
a.nav-link.active {
border: none;
border-color: transparent;
background-color: pvar(--mainBackgroundColor) !important;
}
}
Expand Down
16 changes: 9 additions & 7 deletions client/src/app/shared/shared-main/misc/top-menu-dropdown.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,28 @@
<a *ngIf="menuEntry.routerLink" [routerLink]="menuEntry.routerLink" routerLinkActive="active" class="title-page title-page-settings">{{ menuEntry.label }}</a>

<div *ngIf="!menuEntry.routerLink" ngbDropdown class="parent-entry"
#dropdown="ngbDropdown" (mouseleave)="closeDropdownIfHovered(dropdown)">
Chocobozzz marked this conversation as resolved.
Show resolved Hide resolved
#dropdown="ngbDropdown" autoClose="outside">
Copy link
Owner

Choose a reason for hiding this comment

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

When we click on an element, the dropdown is not automatically closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I explained it also in the description :

Improve Dropdown : active class on active link + remove hover open + close outside click only
make sure the sub-menu is not too long horizontally - less confusing nav and more common

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here it is for 2 things :

  • Accessibility, make sure the dropdown is still openened after click
  • Avoid epileptic effect with the active class onclick (2 events trigged at the same time)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@kimsible for the accessibility, how is this part of WCAG 2.1? for the epilepitc nature, it does not flash per WCAG 2.3.2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Invoking W3C spec would not help us here...but only crushing propositions with an authority argument.
I don't see what you mean and I'm really opened to make any changes, maybe removing the active class and back to the autoClose=true (inside and outsite).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Invoking W3C spec would not help us here

I'm just trying to understand what hurdles we have to care about when it comes to designing the dropdown. If you just say "Accessibility", then I look up what corresponds to it. If you explain the reasoning behind classifying your proposition as more accessible, then we can compare.

Usually dropdowns are kept open for some elements (i.e. nested dropdowns), not just for navigation.

Copy link
Owner

Choose a reason for hiding this comment

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

Accessibility, make sure the dropdown is still openened after click

I don't think users expect a menu to be still opened after a click. Every time we want to go to another page, we have to click 2 times.

Maybe a menu/menubar role could resolve the accessibility issue of dropdown auto close.

Copy link
Contributor Author

@kimsible kimsible Jul 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think users expect a menu to be still opened after a click. Every time we want to go to another page, we have to click 2 times.

This PR goes back to the old effect #3023 but another PR should be created for the accessibility issue.

<span
*ngIf="isInSmallView"
tabindex=0
[ngClass]="{ active: !!suffixLabels[menuEntry.label] }"
(click)="openModal(id)" role="button" class="title-page title-page-settings">
(click)="openModal(id)" (keydown.enter)="openModal(id)"
role="button" class="title-page title-page-settings">
<ng-container i18n>{{ menuEntry.label }}</ng-container>
<ng-container *ngIf="!!suffixLabels[menuEntry.label]"> - {{ suffixLabels[menuEntry.label] }}</ng-container>
</span>

<span
*ngIf="!isInSmallView"
(mouseenter)="openDropdownOnHover(dropdown)" [ngClass]="{ active: !!suffixLabels[menuEntry.label] }" ngbDropdownAnchor
(click)="dropdownAnchorClicked(dropdown)" role="button" class="title-page title-page-settings"
tabindex=0
[ngClass]="{ active: !!suffixLabels[menuEntry.label] }" ngbDropdownAnchor
(click)="dropdownAnchorClicked(dropdown)" (keydown.enter)="dropdownAnchorClicked(dropdown)"
role="button" class="title-page title-page-settings"
>
<ng-container i18n>{{ menuEntry.label }}</ng-container>
<ng-container *ngIf="!!suffixLabels[menuEntry.label]"> - {{ suffixLabels[menuEntry.label] }}</ng-container>
</span>

<div ngbDropdownMenu>
<a *ngFor="let menuChild of menuEntry.children" class="dropdown-item" [ngClass]="{ icon: hasIcons }" [routerLink]="menuChild.routerLink">
<a *ngFor="let menuChild of menuEntry.children" class="dropdown-item" [ngClass]="{ icon: hasIcons, active: suffixLabels[menuEntry.label] === menuChild.label }" [routerLink]="menuChild.routerLink">
<my-global-icon *ngIf="menuChild.iconName" [iconName]="menuChild.iconName" aria-hidden="true"></my-global-icon>

{{ menuChild.label }}
Expand Down
6 changes: 6 additions & 0 deletions client/src/app/shared/shared-main/misc/top-menu-dropdown.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
@import '_variables';
@import '_mixins';

:host {
display: block;
height: $sub-menu-height;
margin-bottom: $sub-menu-margin-bottom;
}

.parent-entry {
span[role=button] {
cursor: pointer;
Expand Down
23 changes: 0 additions & 23 deletions client/src/app/shared/shared-main/misc/top-menu-dropdown.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export class TopMenuDropdownComponent implements OnInit, OnDestroy {
isModalOpened = false
currentMenuEntryIndex: number

private openedOnHover = false
private routeSub: Subscription

constructor (
Expand Down Expand Up @@ -68,32 +67,10 @@ export class TopMenuDropdownComponent implements OnInit, OnDestroy {
if (this.routeSub) this.routeSub.unsubscribe()
}

openDropdownOnHover (dropdown: NgbDropdown) {
this.openedOnHover = true
dropdown.open()

// Menu was closed
dropdown.openChange
.pipe(take(1))
.subscribe(() => this.openedOnHover = false)
}

dropdownAnchorClicked (dropdown: NgbDropdown) {
if (this.openedOnHover) {
this.openedOnHover = false
return
}

return dropdown.toggle()
}

closeDropdownIfHovered (dropdown: NgbDropdown) {
if (this.openedOnHover === false) return

dropdown.close()
this.openedOnHover = false
}

openModal (index: number) {
this.currentMenuEntryIndex = index
this.isModalOpened = true
Expand Down
5 changes: 5 additions & 0 deletions client/src/app/shared/shared-moderation/account-blocklist.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
<h1>
<my-global-icon iconName="user" aria-hidden="true"></my-global-icon>
<ng-container i18n>Muted accounts</ng-container>
</h1>

<p-table
[value]="blockedAccounts" [lazy]="true" [paginator]="totalRecords > 0" [totalRecords]="totalRecords" [rows]="rowsPerPage" [rowsPerPageOptions]="rowsPerPageOptions"
[sortField]="sort.field" [sortOrder]="sort.order" (onLazyLoad)="loadLazy($event)" (onPage)="onPage($event)"
Expand Down
5 changes: 5 additions & 0 deletions client/src/app/shared/shared-moderation/server-blocklist.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
<h1>
<my-global-icon iconName="server" aria-hidden="true"></my-global-icon>
<ng-container i18n>Muted servers</ng-container>
</h1>

<p-table
[value]="blockedServers" [lazy]="true" [paginator]="totalRecords > 0" [totalRecords]="totalRecords" [rows]="rowsPerPage" [rowsPerPageOptions]="rowsPerPageOptions"
[sortField]="sort.field" [sortOrder]="sort.order" (onLazyLoad)="loadLazy($event)" (onPage)="onPage($event)"
Expand Down
Loading