Using active_link_to if defined (falling back to link_to) #22

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

Open

johanandre wants to merge 14 commits into master from use_active_link_to_if_installed

Member

johanandre commented Feb 17, 2017 •

edited by AnteWall

Loading

Cleaning up concat of html (no longer need html_safe on generated output)
Removing inline html (except the icon chevron)
When a block is passed to sidebar_item (for generating a dropdown), an random id is created

closes #10


          - Using active_link_to if defined (falling back to link_to)

6a535b0

- Cleaning up concat of html (no longer need html_safe on generated output)
- Removing inline html (except the icon chevron)
- When a block is passed to sidebar_item (for generating a dropdown), an random id is created

johanandre requested a review from AnteWall

February 17, 2017 18:55

houndci-bot reviewed

View reviewed changes

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

    
                        html = link_to("#{title} <i class='fa fa-chevron-right'></i>".html_safe, url, data: { toggle: 'collapse' })

                        html << content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do

                        concat(link_to("#{title} <i class='fa fa-chevron-right'></i>".html_safe, url, data: {toggle: 'collapse'}))

                        concat(content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do

houndci-bot Feb 17, 2017

Line is too long. [84/80]

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

                       content_tag :li do
-                        html = link_to("#{title} <i class='fa fa-chevron-right'></i>".html_safe, url, data: { toggle: 'collapse' })
-                        html << content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do
+                        concat(link_to("#{title} <i class='fa fa-chevron-right'></i>".html_safe, url, data: {toggle: 'collapse'}))

houndci-bot Feb 17, 2017

Line is too long. [116/80]
Space inside { missing.
Space inside } missing.

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

		if block_given?
		url = "##{(0...20).map { ('a'..'z').to_a[rand(26)] }.join}" unless url.present?

houndci-bot Feb 17, 2017

Line is too long. [87/80]

lib/sigbit_admin_rails/helpers/sidebar_helper.rb

@@ @@ -13,17 +13,29 @@ def sidebar(*args, &block) @@
                     end
                   end
-                  def sidebar_item(title, url, &block)
+                  def sidebar_item(title, url = nil, &block)

houndci-bot Feb 17, 2017

Assignment Branch Condition size for sidebar_item is too high. [16.16/15]
Method has too many lines. [13/10]
Unused method argument - block. If it's necessary, use _ or _block as an argument name to indicate that it won't be used.


          Small fixes

be035c9

houndci-bot reviewed

View reviewed changes

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

                       content_tag :li do
-                        html = link_to("#{title} <i class='fa fa-chevron-right'></i>".html_safe, url, data: { toggle: 'collapse' })
-                        html << content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do
+                        concat(link_to("#{title} #{dropdown_arrow_icon}".html_safe, url, data: { toggle: 'collapse' }))

houndci-bot Feb 17, 2017

Line is too long. [105/80]

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

		if block_given?
		url = "##{(0...20).map { ('a'..'z').to_a.sample }.join }" unless url.present?

houndci-bot Feb 17, 2017

Space inside string interpolation detected.
Line is too long. [85/80]

johanandre added 3 commits

February 17, 2017 21:55


          Fixing long line

afc1314


          Refactoring to method

e79507e


          Reducing method size

a39d3da

houndci-bot reviewed

View reviewed changes

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

    
                        html = link_to("#{title} <i class='fa fa-chevron-right'></i>".html_safe, url, data: { toggle: 'collapse' })

                        html << content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do

                        concat(link_to("#{title} #{dropdown_arrow_icon}".html_safe, url, data: {toggle: 'collapse'}))

                        concat(content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do

houndci-bot Feb 17, 2017

Line is too long. [84/80]

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

                       content_tag :li do
-                        html = link_to("#{title} <i class='fa fa-chevron-right'></i>".html_safe, url, data: { toggle: 'collapse' })
-                        html << content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do
+                        concat(link_to("#{title} #{dropdown_arrow_icon}".html_safe, url, data: {toggle: 'collapse'}))

houndci-bot Feb 17, 2017

Line is too long. [103/80]
Space inside { missing.
Space inside } missing.

lib/sigbit_admin_rails/helpers/sidebar_helper.rb

@@ @@ -13,17 +13,36 @@ def sidebar(*args, &block) @@
                     end
                   end
-                  def sidebar_item(title, url, &block)
+                  def sidebar_item(title, url = nil, &block)

houndci-bot Feb 17, 2017

Method has too many lines. [11/10]
Unused method argument - block. If it's necessary, use _ or _block as an argument name to indicate that it won't be used.


          Space inside {}

0d586f6

houndci-bot reviewed

View reviewed changes

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

                       content_tag :li do
-                        html = link_to("#{title} <i class='fa fa-chevron-right'></i>".html_safe, url, data: { toggle: 'collapse' })
-                        html << content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do
+                        concat(link_to("#{ title } #{ dropdown_arrow_icon }".html_safe, url, data: { toggle: 'collapse' }))

houndci-bot Feb 17, 2017

Space inside string interpolation detected.
Line is too long. [109/80]


          Splitting lines

060b8c6

houndci-bot reviewed

View reviewed changes

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

+                        concat(
+                            link_to("#{ title } #{ dropdown_arrow_icon }".html_safe, url, data: {toggle: 'collapse'})
+                        )
+                        concat(content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do

houndci-bot Feb 17, 2017

Line is too long. [84/80]

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

-                        html = link_to("#{title} <i class='fa fa-chevron-right'></i>".html_safe, url, data: { toggle: 'collapse' })
-                        html << content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do
+                        concat(
+                            link_to("#{ title } #{ dropdown_arrow_icon }".html_safe, url, data: {toggle: 'collapse'})

houndci-bot Feb 17, 2017

Indent the first parameter one step more than the start of the previous line.
Space inside string interpolation detected.
Line is too long. [103/80]
Space inside { missing.
Space inside } missing.


          Fiddling with line length

37e2339

houndci-bot reviewed

View reviewed changes

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

-                        html = link_to("#{title} <i class='fa fa-chevron-right'></i>".html_safe, url, data: { toggle: 'collapse' })
-                        html << content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do
+                        concat(
+                          link_to("#{ title } #{ dropdown_arrow_icon }".html_safe, url, data: {toggle: 'collapse'})

houndci-bot Feb 17, 2017

Space inside string interpolation detected.
Line is too long. [101/80]
Space inside { missing.
Space inside } missing.


          Shortening more to please Hound

cb01e52

houndci-bot reviewed

View reviewed changes

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

-                        html = link_to("#{title} <i class='fa fa-chevron-right'></i>".html_safe, url, data: { toggle: 'collapse' })
-                        html << content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do
+                        concat(
+                          link_to("#{ title } #{ arrow_icon }".html_safe, url, data: {toggle: 'collapse'})

houndci-bot Feb 17, 2017

Space inside string interpolation detected.
Space inside { missing.
Line is too long. [92/80]
Space inside } missing.


          More improvments

b68097f

houndci-bot reviewed

View reviewed changes

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

+                          link_to("#{ title } #{ arrow_icon }".html_safe,
+                                  url, data: { toggle: 'collapse' })
+                        )
+                        concat(content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do

houndci-bot Feb 17, 2017

Line is too long. [84/80]

lib/sigbit_admin_rails/helpers/sidebar_helper.rb

@@ @@ -13,17 +13,39 @@ def sidebar(*args, &block) @@
                     end
                   end
-                  def sidebar_item(title, url, &block)
+                  def sidebar_item(title, url = nil, &block)

houndci-bot Feb 17, 2017

Method has too many lines. [14/10]
Unused method argument - block. If it's necessary, use _ or _block as an argument name to indicate that it won't be used.


          Fixing long line

4a11b59

houndci-bot reviewed

View reviewed changes

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

+                        )
+                        concat(
+                          content_tag(:ul, class: 'submenu collapse',
+                                      id: url.delete('#')) do

houndci-bot Feb 17, 2017

Align the elements of a hash literal if they span more than one line.

lib/sigbit_admin_rails/helpers/sidebar_helper.rb Outdated

-                        end
-                        html.html_safe
+                        concat(
+                          link_to("#{ title } #{ arrow_icon }".html_safe,

houndci-bot Feb 17, 2017

Space inside string interpolation detected.

lib/sigbit_admin_rails/helpers/sidebar_helper.rb

@@ @@ -13,17 +13,42 @@ def sidebar(*args, &block) @@
                     end
                   end
-                  def sidebar_item(title, url, &block)
+                  def sidebar_item(title, url = nil, &block)

houndci-bot Feb 17, 2017

Method has too many lines. [17/10]
Unused method argument - block. If it's necessary, use _ or _block as an argument name to indicate that it won't be used.

johanandre added 3 commits

February 17, 2017 22:18


          Align hash

86b4f6b


          Fixing closing i-tag

984eb9c


          Restore long lines due to horrible code

3a7f1f2

houndci-bot reviewed

View reviewed changes

lib/sigbit_admin_rails/helpers/sidebar_helper.rb

    
                        html = link_to("#{title} <i class='fa fa-chevron-right'></i>".html_safe, url, data: { toggle: 'collapse' })

                        html << content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do

                        concat(link_to("#{ title } #{ arrow_icon }".html_safe, url, data: { toggle: 'collapse' }))

                        concat(content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do

houndci-bot Feb 17, 2017

Line is too long. [84/80]

lib/sigbit_admin_rails/helpers/sidebar_helper.rb

                       content_tag :li do
-                        html = link_to("#{title} <i class='fa fa-chevron-right'></i>".html_safe, url, data: { toggle: 'collapse' })
-                        html << content_tag(:ul, class: 'submenu collapse', id: url.delete('#')) do
+                        concat(link_to("#{ title } #{ arrow_icon }".html_safe, url, data: { toggle: 'collapse' }))

houndci-bot Feb 17, 2017

Space inside string interpolation detected.
Line is too long. [100/80]

lib/sigbit_admin_rails/helpers/sidebar_helper.rb

@@ @@ -4,26 +4,45 @@ def sidebar(*args, &block) @@
                     options = args.extract_options!
                     if block_given?
                       content_tag :div, id: 'sidebar-wrapper' do
-                        html = link_to '', options[:brand_url], class: 'sidebar-brand hidden-sm-down'
-                        html << content_tag(:ul, class: 'sidebar-nav') do
+                        concat(link_to '', options[:brand_url], class: 'sidebar-brand hidden-sm-down')

houndci-bot Feb 17, 2017

Add parentheses to nested method call link_to '', options[:brand_url], class: 'sidebar-brand hidden-sm-down'.
Line is too long. [88/80]

Member Author

johanandre commented Feb 18, 2017

I fiddled around to please Hound but ended up in some shitty looking code, thats why there is a bunch of commits changing stuff back and forth... :)

AnteWall suggested changes

View reviewed changes

Contributor

AnteWall left a comment

Its better then it was before, however the biggest problem we had before is that we can't customize it. It would be nice if we could modify the classes that its append. (i.e the nav-link etc...)

Perhaps change them completely by sending in our own class.

Member Author

johanandre commented Feb 21, 2017

Well, since it moves the sidebar helpers to a working state (from not working that great), although not adding everything, I think we should merge it anyway. Of course I'm open for solving the customization in a later pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet