-
Notifications
You must be signed in to change notification settings - Fork 191
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
Hera/add main layout #1318
base: develop
Are you sure you want to change the base?
Hera/add main layout #1318
Conversation
e0f4567
to
ce25368
Compare
</a> | ||
<ul class="dropdown-menu dropdown-menu-end" aria-labelledby="configurationDropdown"> | ||
<li><%= link_to 'Configure Integrations', main_app.project_configurations_path(current_project), class: 'dropdown-item' %></li> | ||
<li><%= link_to "Manage Tags", project_tags_path(current_project), class: 'dropdown-item' %></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<li><%= link_to "Manage Tags", project_tags_path(current_project), class: 'dropdown-item' %></li> | |
<li><%= link_to "Manage Tags", main_app.project_tags_path(current_project), class: 'dropdown-item' %></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need this for when we access /validations or /export
function submitSearch($form) { | ||
if ($form.find('[data-behavior~=search-query]').val() !== '') { | ||
$form.submit(); | ||
$form.find('[data-behavior~=search-query]').val('Searching...'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is breaking the search functionality
}); | ||
|
||
$('[data-behavior~=search-query]').on('keypress', function (e) { | ||
if (e.which === 13) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we make the magic number a variable like we did with mobileBreakpoint
in sidebar.js
<%= require_asset "#{plugin_path}/manifests/tylium" %> | ||
<%= require_asset "#{plugin_path}/manifests/hera" %> | ||
<% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<%= require_asset "#{plugin_path}/manifests/tylium" %> | |
<%= require_asset "#{plugin_path}/manifests/hera" %> | |
<% | |
<%= require_asset "#{plugin_path}/manifests/tylium" %> | |
<%= require_asset "#{plugin_path}/manifests/hera" %> | |
<% |
@@ -0,0 +1,7 @@ | |||
class StylesHeraController < AuthenticatedController |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be called StylesHereController
?
Can we rename it to HeraController
instead and have hera/index
? or styles/hera/index
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is carried over from StylesTyliumController
. Happy to change this but it's out of scope here.
@@ -0,0 +1,71 @@ | |||
<nav class="navbar navbar-expand-lg project-nav"> | |||
<div class="container-fluid justify-content-end p-lg-0"> | |||
<div class="collapse navbar-collapse dual-nav" id="project_navbar" data-behavior="navbar-collapse"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto with the id. it's in a different format than the one above, if we need them let's make the case consistent
<% end %> | ||
</li> | ||
<li class="nav-item dropdown ms-0"> | ||
<a href="javascript:void(0)" class="nav-link dropdown-toggle no-caret-xl" id="configurationDropdown" title="Configuration" role="button" data-bs-toggle="dropdown" aria-expanded="false"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id needed?
@@ -0,0 +1,19 @@ | |||
<li class="nav-item dropdown"> | |||
<a href="javascript:void(0)" class="nav-link dropdown-toggle" data-bs-toggle="dropdown" id="toolsDropdown"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<a href="javascript:void(0)" class="nav-link dropdown-toggle" data-bs-toggle="dropdown" id="toolsDropdown"> | |
<a href="javascript:void(0)" class="nav-link dropdown-toggle" data-bs-toggle="dropdown"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed?
Summary
Replacing the
Tylium
layout with the initial implementation of theHera
layout. This brings many new improvements such as the navigation and sidebar redesigns. The layout is structured in a more modern way under the hood, preventing us from having to use workarounds or patches for certain functionality/interactions as we had to withTylium
.This will serve as the foundation for further layout improvements that will be implemented incrementally.
Check List