-
Notifications
You must be signed in to change notification settings - Fork 900
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
Convert old accordions to Bootstrap #1558
Conversation
This is the initial conversion of listnavs that @skateman can follow. @skateman , @martinpovolny or @dclarizio , please review. |
- if !@default_search.blank? && @default_search.name == search.name | ||
- search_name = search.description | ||
- else | ||
- search_name = search.description |
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.
why do we have the if
here when the positive branch is the same as the negative one?
Checked commit https://github.com/epwinchell/manageiq/commit/271802d249ccdfe9c94dbd27844d7915ae622f0b with rubocop 0.27.1 |
- else | ||
- bgcolor = "" | ||
- li_class = "" | ||
- elsif @edit && @edit[:expression] && ((@edit[:expression][:selected] && @edit[:expression][:selected][:id] == 0 && search.id.to_i == 0) || (@edit[:expression][:selected] && @edit[:expression][:selected][:name] == search.name && !@edit[:custom_search])) |
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.
that programming inside haml
looks pretty ugly, we shall get this out of the view, but probably as a part of some other PR
I've done a very quick review and it looks quite good, just some indentation issues. @epwinchell i suggest you to use the haml-lint gem to find indentation and other issues. You can integrate it to your editor or just run it from the root directory of the repo like this: |
Issue #1557
This is the initial conversion of listnavs that @skateman can follow. @skateman , @martinpovolny or @dclarizio , please review.
Before:
After: