Skip to content

Commit cc90002

Browse files
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 cc90002

File tree

27 files changed

+242
-122
lines changed

27 files changed

+242
-122
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: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,24 @@
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
32+
z-index: 1
3333

34-
#menu-toggler--opener
34+
#menu-toggle--collapse-button
3535
margin-right: 5px
3636

3737
.Button-visual
3838
color: var(--main-menu-font-color)
3939

4040
#wrapper
4141
.hidden-navigation
42-
// TODO: Add angular breadcrumb selector once it exists
43-
.PageHeader-contextBar
42+
// Push the breadcrumb to the right when the sidebar is collapsed
43+
.PageHeader-contextBar,
44+
.op-breadcrumbs
4445
margin-left: 30px
4546

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

app/components/open_project/common/submenu_component.sass

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@
55

66
&--search
77
position: sticky
8-
// Header item + 10px padding + 77px project selector
9-
top: calc(var(--main-menu-item-height) + 10px + 77px)
8+
top: 0
109
z-index: 1
1110
background: var(--main-menu-bg-color)
1211
padding: 12px var(--main-menu-x-spacing)

app/views/layouts/base.html.erb

Lines changed: 33 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,36 @@ 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+
<div class="main-menu--top-container">
105+
<%= render(Primer::OpenProject::FlexLayout.new(align_items: :center, pt: 3)) do |flex|
106+
flex.with_column(overflow: :hidden, ml: 2) do
107+
render_projects_top_menu_node
108+
end
109+
flex.with_column(text_align: :right, flex: :auto) do
110+
render(OpenProject::Common::MainMenuToggleComponent.new(expanded: true))
111+
end
112+
end %>
114113

115-
<%= render(Primer::BaseComponent.new(tag: :hr,
116-
classes: "main-menu--projects-divider",
117-
role: :presentation,
118-
aria: {
119-
hidden: true
120-
})) %>
114+
<%= render(
115+
Primer::BaseComponent.new(
116+
tag: :hr,
117+
classes: "main-menu--projects-divider",
118+
role: :presentation,
119+
aria: {
120+
hidden: true
121+
}
122+
)
123+
) %>
124+
</div>
121125
<%= main_menu %>
122126
<%= content_for :main_menu %>
123127
<%= call_hook :view_layouts_base_main_menu %>
@@ -139,11 +143,11 @@ See COPYRIGHT and LICENSE files for more details.
139143
<section data-augmented-model-wrapper
140144
data-modal-initialize-now="true"
141145
data-modal-class-name="onboarding-modal">
142-
<%= render partial: '/onboarding/configuration_modal' %>
146+
<%= render partial: "/onboarding/configuration_modal" %>
143147
</section>
144148
<% end %>
145149
<%= content_tag :div,
146-
id: 'content',
150+
id: "content",
147151
class: "#{initial_classes} #{'content--split' if content_for?(:content_body_right)}" do %>
148152
<h1 class="hidden-for-sighted accessibility-helper"><%= t(:label_content) %></h1>
149153
<% if content_for?(:content_header) %>
@@ -153,7 +157,7 @@ See COPYRIGHT and LICENSE files for more details.
153157
<% end %>
154158

155159
<div id="content-body">
156-
<%= render(OpenProject::Common::MainMenuTogglerComponent.new(opened: false)) %>
160+
<%= render(OpenProject::Common::MainMenuToggleComponent.new(expanded: false)) %>
157161
<%= content_for :content_body %>
158162
<% unless local_assigns[:no_layout_yield] %>
159163
<%= 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/features/bim/ifc_models/pages/viewer/styles/generic.sass

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@
3737
.op-breadcrumbs
3838
margin-left: 20px
3939

40+
#wrapper .hidden-navigation
41+
.op-breadcrumbs
42+
margin-left: 45px
43+
4044
[data-name="bim"] .main-menu--children
4145
overflow-x: hidden !important
4246

frontend/src/app/shared/components/breadcrumbs/op-breadcrumbs.component.sass

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
@import "helpers"
22

33
.op-breadcrumbs
4+
// Match the height of the Primer breadcrumb which itself takes the height of the small action button it can have on mobile
5+
height: var(--control-small-size)
6+
line-height: var(--control-small-size)
7+
48
.op-breadcrumbs--items
59
margin-left: 0
610
.op-breadcrumbs--item

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)

0 commit comments

Comments
 (0)