Skip to content

Commit 9181511

Browse files
HDingerHDinger
committed
Take care that the main menu resizer toggles when the menu is collapsed/expanded
Clean-up sass code Avoid that the type selector in the WP from is scrolled two far when being activated. Currently, it is scrolled twice (one time when activating it and once again when selecting an item). Thus the dropdown overlaps the actual input Move logo to the left side Make naming consistent for MainMenuToggleComponent and add a component test Let menu toggle button scroll with the rest of the page Move modules menu to the left and show it on mobile
1 parent d983851 commit 9181511

File tree

21 files changed

+208
-110
lines changed

21 files changed

+208
-110
lines changed

‎app/components/_index.sass

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
@import "open_project/common/attribute_help_text_component"
77
@import "open_project/common/attribute_label_component"
88
@import "open_project/common/submenu_component"
9-
@import "open_project/common/main_menu_toggler_component"
9+
@import "open_project/common/main_menu_toggle_component"
1010
@import "projects/row_component"
1111
@import "projects/phases/hover_card_component"
1212
@import "shares/invite_user_form_component"

‎app/components/open_project/common/main_menu_toggler_component.rb renamed to ‎app/components/open_project/common/main_menu_toggle_component.rb

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,47 +30,49 @@
3030
#
3131
module OpenProject
3232
module Common
33-
class MainMenuTogglerComponent < ApplicationComponent
34-
def initialize(opened:)
35-
super
33+
class MainMenuToggleComponent < ApplicationComponent
34+
def initialize(expanded:)
35+
super()
3636

37-
@opened = opened
37+
@expanded = expanded
3838
end
3939

40+
def call
41+
render(Primer::Beta::IconButton.new(icon:,
42+
id:,
43+
aria: { label: aria_label },
44+
scheme:,
45+
size:,
46+
data:))
47+
end
48+
49+
private
50+
4051
def icon
41-
@opened ? "sidebar-expand" : :"sidebar-collapse"
52+
@expanded ? "sidebar-expand" : :"sidebar-collapse"
4253
end
4354

4455
def id
45-
"menu-toggler--#{@opened ? 'opener' : 'closer'}"
56+
"menu-toggle--#{@expanded ? 'collapse-button' : 'expand-button'}"
4657
end
4758

4859
def aria_label
49-
@opened ? I18n.t("js.label_hide_project_menu") : I18n.t("js.label_expand_project_menu")
60+
@expanded ? I18n.t("js.label_hide_project_menu") : I18n.t("js.label_expand_project_menu")
5061
end
5162

5263
def scheme
5364
:invisible
5465
end
5566

5667
def size
57-
@opened ? :medium : :small
68+
@expanded ? :medium : :small
5869
end
5970

6071
def data
6172
{
62-
action: "click->menus--main-toggler#toggleNavigation"
73+
action: "click->menus--main-toggle#toggleNavigation"
6374
}
6475
end
65-
66-
def call
67-
render(Primer::Beta::IconButton.new(icon:,
68-
id:,
69-
aria: { label: aria_label },
70-
scheme:,
71-
size:,
72-
data:))
73-
end
7476
end
7577
end
7678
end

‎app/components/open_project/common/main_menu_toggler_component.sass renamed to ‎app/components/open_project/common/main_menu_toggle_component.sass

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,11 @@
2626
// See COPYRIGHT and LICENSE files for more details.
2727
//++
2828
29-
#menu-toggler--closer
29+
#menu-toggle--expand-button
3030
position: absolute
31-
top: 14px
3231
left: 11px
3332

34-
#menu-toggler--opener
33+
#menu-toggle--collapse-button
3534
margin-right: 5px
3635

3736
.Button-visual
@@ -42,7 +41,9 @@
4241
// TODO: Add angular breadcrumb selector once it exists
4342
.PageHeader-contextBar
4443
margin-left: 30px
44+
.PageHeader-contextBar
45+
height: 28px // ToDo: Remove once moved to primer repo
4546

4647
&:not(.hidden-navigation)
47-
#menu-toggler--closer
48+
#menu-toggle--expand-button
4849
display: none

‎app/views/layouts/base.html.erb

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ See COPYRIGHT and LICENSE files for more details.
3030
<html xmlns="http://www.w3.org/1999/xhtml"
3131
lang="<%= I18n.locale.to_s %>"
3232
xml:lang="<%= I18n.locale.to_s %>"
33-
class="<%= 'in_modal' unless show_decoration %>">
33+
class="<%= "in_modal" unless show_decoration %>">
3434
<head>
35-
<%= render partial: 'layouts/common_head' %>
35+
<%= render partial: "layouts/common_head" %>
3636
<!-- project specific tags -->
3737
<%= call_hook :view_layouts_base_html_head %>
3838
<!-- page specific tags -->
@@ -43,13 +43,13 @@ See COPYRIGHT and LICENSE files for more details.
4343
</head>
4444
<%= content_tag :body,
4545
class: "#{body_css_classes} __overflowing_element_container __overflowing_body",
46-
data: body_data_attributes(local_assigns) do %>
47-
<%= render partial: 'warning_bar/warning_bar' %>
46+
data: body_data_attributes(local_assigns) do %>
47+
<%= render partial: "warning_bar/warning_bar" %>
4848
<noscript>
4949
<div class="top-shelf">
5050
<p>
5151
<span>
52-
<%= op_icon 'icon3 icon-warning' %>
52+
<%= op_icon "icon3 icon-warning" %>
5353
<strong><%= t(:noscript_heading) %></strong>
5454
</span>
5555
<%= t(:noscript_description) %>
@@ -69,15 +69,15 @@ See COPYRIGHT and LICENSE files for more details.
6969
<% initial_classes = initial_menu_classes(side_displayed, show_decoration) %>
7070
<div id="wrapper" style="<%= initial_menu_styles(side_displayed) %>" class="<%= initial_classes %>">
7171
<% if show_decoration %>
72-
<header class="op-app-header<%= ' op-app-header_development' if OpenProject::Configuration.development_highlight_enabled? %>">
72+
<header class="op-app-header<%= " op-app-header_development" if OpenProject::Configuration.development_highlight_enabled? %>">
7373
<div class="op-app-header--start">
7474
<h1 class="hidden-for-sighted">
7575
<%= t(:label_top_menu) %>
7676
</h1>
7777
<a href="" class="hidden-for-sighted skip-navigation-link"
7878
id="skip-navigation--content"
79-
aria-label="<%= I18n.t('js.work_packages.jump_marks.label_content') %>">
80-
<%= I18n.t('js.work_packages.jump_marks.content') %>
79+
aria-label="<%= I18n.t("js.work_packages.jump_marks.label_content") %>">
80+
<%= I18n.t("js.work_packages.jump_marks.content") %>
8181
</a>
8282
<%= render_top_menu_left %>
8383
</div>
@@ -92,32 +92,34 @@ See COPYRIGHT and LICENSE files for more details.
9292
<% end %>
9393
<div id="main"
9494
class="<%= initial_classes %>"
95-
data-controller="menus--main-toggler"
96-
data-application-target="dynamic">
95+
data-controller="menus--main-toggle">
9796
<% if (side_displayed && show_decoration) %>
9897
<div
9998
id="main-menu"
10099
class="main-menu"
101-
data-controller="menus--main"
102-
>
100+
data-controller="menus--main">
103101
<h1 class="hidden-for-sighted"><%= t(:label_main_menu) %></h1>
104102
<opce-main-menu-resizer></opce-main-menu-resizer>
105103
<div id="menu-sidebar">
106-
<%= render(Primer::OpenProject::FlexLayout.new(align_items: :center, mt: 2)) do |flex|
107-
flex.with_column(overflow: :hidden, ml: 2) do
108-
render_projects_top_menu_node
109-
end
110-
flex.with_column(text_align: :right, flex: :auto) do
111-
render(OpenProject::Common::MainMenuTogglerComponent.new(opened: true))
112-
end
113-
end %>
104+
<%= render(Primer::OpenProject::FlexLayout.new(align_items: :center, pt: 3, classes: "main-menu--project-selector-container")) do |flex|
105+
flex.with_column(overflow: :hidden, ml: 2) do
106+
render_projects_top_menu_node
107+
end
108+
flex.with_column(text_align: :right, flex: :auto) do
109+
render(OpenProject::Common::MainMenuToggleComponent.new(expanded: true))
110+
end
111+
end %>
114112

115-
<%= render(Primer::BaseComponent.new(tag: :hr,
116-
classes: "main-menu--projects-divider",
117-
role: :presentation,
118-
aria: {
119-
hidden: true
120-
})) %>
113+
<%= render(
114+
Primer::BaseComponent.new(
115+
tag: :hr,
116+
classes: "main-menu--projects-divider",
117+
role: :presentation,
118+
aria: {
119+
hidden: true
120+
}
121+
)
122+
) %>
121123
<%= main_menu %>
122124
<%= content_for :main_menu %>
123125
<%= call_hook :view_layouts_base_main_menu %>
@@ -139,11 +141,11 @@ See COPYRIGHT and LICENSE files for more details.
139141
<section data-augmented-model-wrapper
140142
data-modal-initialize-now="true"
141143
data-modal-class-name="onboarding-modal">
142-
<%= render partial: '/onboarding/configuration_modal' %>
144+
<%= render partial: "/onboarding/configuration_modal" %>
143145
</section>
144146
<% end %>
145147
<%= content_tag :div,
146-
id: 'content',
148+
id: "content",
147149
class: "#{initial_classes} #{'content--split' if content_for?(:content_body_right)}" do %>
148150
<h1 class="hidden-for-sighted accessibility-helper"><%= t(:label_content) %></h1>
149151
<% if content_for?(:content_header) %>
@@ -153,7 +155,7 @@ See COPYRIGHT and LICENSE files for more details.
153155
<% end %>
154156

155157
<div id="content-body">
156-
<%= render(OpenProject::Common::MainMenuTogglerComponent.new(opened: false)) %>
158+
<%= render(OpenProject::Common::MainMenuToggleComponent.new(expanded: false)) %>
157159
<%= content_for :content_body %>
158160
<% unless local_assigns[:no_layout_yield] %>
159161
<%= yield %>

‎frontend/src/app/core/main-menu/main-menu-toggle.service.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,16 +49,14 @@ export class MainMenuToggleService {
4949

5050
@InjectField() currentProject:CurrentProjectService;
5151

52-
private global = (window as any);
53-
5452
private htmlNode = document.getElementsByTagName('html')[0];
5553

5654
private mainMenu = jQuery('#main-menu')[0]; // main menu, containing sidebar and resizer
5755

5856
// Notes all changes of the menu size (currently needed in wp-resizer.component.ts)
59-
private changeData = new BehaviorSubject<any>({});
60-
57+
private changeData = new BehaviorSubject<number|undefined>(undefined);
6158
public changeData$ = this.changeData.asObservable();
59+
6260
private wasHiddenDueToResize = false;
6361

6462
private wasCollapsedByUser = false;
@@ -140,11 +138,13 @@ export class MainMenuToggleService {
140138

141139
public closeMenu():void {
142140
this.setWidth(0);
141+
this.changeData.next(0);
143142
jQuery('.searchable-menu--search-input').blur();
144143
}
145144

146145
public openMenu():void {
147146
this.setWidth(this.defaultWidth);
147+
this.changeData.next(this.defaultWidth);
148148
}
149149

150150
public setWidth(width?:number):void {

‎frontend/src/app/shared/components/header-project-select/header-project-select.component.html

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,18 @@
99
id="projects-menu"
1010
accesskey="5"
1111
aria-haspopup="true"
12-
class="op-menu--item op-project-select--trigger-button"
12+
class="op-project-select--trigger-button"
1313
type="button"
1414
slot="trigger"
1515
(click)="toggleDropModal()"
1616
data-test-selector="op-projects-menu"
1717
>
1818
<span
19-
class="op-menu--item-title">
20-
<span
21-
class="ellipsis"
22-
[textContent]="currentProjectName()"
23-
[attr.title]="currentProjectName()">
24-
</span>
19+
class="ellipsis"
20+
[textContent]="currentProjectName()"
21+
[attr.title]="currentProjectName()">
2522
</span>
26-
<i class="op-menu--item-dropdown-indicator button--dropdown-indicator"></i>
23+
<i class="button--dropdown-indicator"></i>
2724
</button>
2825

2926
<ng-container slot="body">

‎frontend/src/app/shared/components/header-project-select/header-project-select.component.sass

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,17 @@
1717
color: var(--fgColor-muted, var(--color-fg-subtle))
1818

1919
&--trigger-button
20-
height: var(--main-menu-item-height)
20+
display: flex
21+
align-items: center
22+
height: var(--control-medium-size)
2123
color: var(--main-menu-font-color)
24+
font-weight: var(--base-text-weight-semibold)
25+
padding: 0 8px
2226
border: 1px solid transparent
23-
padding: 0 var(--main-menu-x-spacing-small)
2427
border-radius: var(--borderRadius-medium)
2528
background-color: transparent
2629
width: 100%
30+
31+
.button--dropdown-indicator
32+
&:before
33+
margin-left: var(--base-size-8)

‎frontend/src/app/shared/components/resizer/resizer/main-menu-resizer.component.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ import { ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, OnIn
3030
import { ResizeDelta } from 'core-app/shared/components/resizer/resizer.component';
3131
import { UntilDestroyedMixin } from 'core-app/shared/helpers/angular/until-destroyed.mixin';
3232
import { MainMenuToggleService } from 'core-app/core/main-menu/main-menu-toggle.service';
33+
import {
34+
debounceTime,
35+
distinctUntilChanged,
36+
} from 'rxjs/operators';
3337

3438
@Component({
3539
selector: 'opce-main-menu-resizer',
@@ -44,7 +48,7 @@ import { MainMenuToggleService } from 'core-app/core/main-menu/main-menu-toggle.
4448
<button
4549
class="spot-link main-menu--navigation-toggler"
4650
[attr.title]="toggleTitle"
47-
[class.open]="toggleService.showNavigation"
51+
[class.open]="isOpen"
4852
(click)="toggleService.toggleNavigation($event)"
4953
>
5054
<span class="resize-handle"><svg op-resizer-vertical-lines-icon size="small"></svg></span>
@@ -58,14 +62,16 @@ import { MainMenuToggleService } from 'core-app/core/main-menu/main-menu-toggle.
5862
export class MainMenuResizerComponent extends UntilDestroyedMixin implements OnInit {
5963
public toggleTitle:string;
6064

61-
private resizeEvent:string;
65+
private resizeEvent:string = 'main-menu-resize';
6266

6367
private elementWidth:number;
6468

6569
private mainMenu = jQuery('#main-menu')[0];
6670

6771
public moving = false;
6872

73+
public isOpen:boolean;
74+
6975
constructor(
7076
readonly toggleService:MainMenuToggleService,
7177
readonly cdRef:ChangeDetectorRef,
@@ -75,7 +81,19 @@ export class MainMenuResizerComponent extends UntilDestroyedMixin implements OnI
7581
}
7682

7783
ngOnInit() {
78-
this.resizeEvent = 'main-menu-resize';
84+
this.isOpen = this.toggleService.showNavigation;
85+
86+
// Listen on sidebar changes and toggle resizer classes, if necessary
87+
this.toggleService.changeData$
88+
.pipe(
89+
distinctUntilChanged(),
90+
this.untilDestroyed(),
91+
debounceTime(50),
92+
)
93+
.subscribe(() => {
94+
this.isOpen = this.toggleService.showNavigation;
95+
this.cdRef.detectChanges();
96+
});
7997
}
8098

8199
public resizeStart() {

0 commit comments

Comments
 (0)