tag page tab overhaul, removing sidebar, styling/spacing tweaks

[jywarren]

Hi @Tlazypanda -- i'm making some changes from #6307 here, and it may need some aria fixes if you're able to help out? Thank you!!

More soon, this is just my first commit.

Compact view:

Larger view:

And largest layout:

Do we prefer that the text of the tabs is all black? I guess that makes sense, but it's an easy fix.

UPDATE:

OK, i made all tab text black (dark grey) and adjusted the spacing below the main image. Tidier:

[codecov]

Codecov Report

Merging #8046 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #8046   +/-   ##
=======================================
  Coverage   82.49%   82.49%           
=======================================
  Files          99       99           
  Lines        5737     5737           
=======================================
  Hits         4733     4733           
  Misses       1004     1004           

[jywarren]

OK, this needs test tweaks:

ERROR["test_viewing_the_dropdown_menu", #<Minitest::Reporters::Suite:0x000000000468a8e0 @name="TagTest">, 84.03439879500002]
 test_viewing_the_dropdown_menu#TagTest (84.03s)
Minitest::UnexpectedError:         Capybara::ElementNotFound: Unable to find link or button "by type"
            test/system/tags_controller_test.rb:21:in `block in <class:TagTest>'
[Screenshot]: tmp/screenshots/failures_test_ban_and_unban_authors_in_spam2.png
ERROR["test_ban_and_unban_authors_in_spam2", #<Minitest::Reporters::Suite:0x00007f5080091758 @name="SpamTest">, 163.256268584]
 test_ban_and_unban_authors_in_spam2#SpamTest (163.26s)
Minitest::UnexpectedError:         Capybara::ExpectationNotMet: expected to find css "a[href='/ban/7'" but there were no matches
            test/system/spam2_test.rb:51:in `block in <class:SpamTest>'

and

 FAIL["test_should_choose_i18n_for_tag/show", #<Minitest::Reporters::Suite:0x00000000130a7ba8 @name="I18nTest">, 122.19089364400003]
 test_should_choose_i18n_for_tag/show#I18nTest (122.19s)
        <Maps> expected but was
        <Public Lab>..
        Expected 0 to be >= 1.
        test/integration/I18n_test.rb:436:in `block (2 levels) in <class:I18nTest>'
        test/integration/I18n_test.rb:431:in `each'
        test/integration/I18n_test.rb:431:in `block in <class:I18nTest>'

[Tlazypanda]

@jywarren Yeah sure would love to help! 🎉 So just to be clear I need to the code added be more accessible by adding aria labels etc right? Also should I pull from this branch and add my commit to this pr or like separate? Thanks ✌️

[jywarren]

If you could open a PR against this branch that'd be awesome. Then after I fix the tests I can pull your code in and merge all together.

[jywarren]

Thank you!!!!

[ebarry]

This looks beautiful!!!

[jywarren]

Fixed 2, and i think the third (spam related) is non-deterministic and possibly unrelated... let's see!

[jywarren]

Hmm. @keshavsethi I am seeing an error here:

https://github.com/jywarren/plots2/blob/d68bee4bc59509fd985aa61661d7750de9441f1d/test/system/spam2_test.rb#L44

The error output is above. Do you have any idea what went wrong? Thank you!!!

[keshavsethi]

Hmm. @keshavsethi I am seeing an error here:

https://github.com/jywarren/plots2/blob/d68bee4bc59509fd985aa61661d7750de9441f1d/test/system/spam2_test.rb#L44

The error output is above. Do you have any idea what went wrong? Thank you!!!

@jywarren System tests of this PR are fine. Can you please check it? Thanks

[Tlazypanda]

Hey @jywarren I ran the changed pages through WAVE and there is only one tiny change that has to been done

(show.html.erb line 60 err:empty link) <a href="https://publiclab.org/wiki/contributors"><i class="fa fa-question-circle-o" aria-hidden="true"></i></a>

Maybe we can open this as an fto? ✌️

[Tlazypanda]

@jywarren The changes look great!! 😍

[ebarry]

This looks fantastic, thank you so much!!!

[ebarry]

Oh one question about the ambiguity you asked in #6307 -- i support removing the line of "stats", i agree "we don't need that little stats display at all anymore" and that we should remove it.

At a later time, i will probably request that we think about other ways to assess tags in addition to viewing them by # of subscribers as shown here: https://publiclab.org/stats/subscriptions . Perhaps a view that allows sorting and re-ordering of tags by # of contributors, and by total content posted in a tag. Perhaps some way that site mods can manually group and then see all water-related tags together.... like this wikimedia "category tree": https://www.appropedia.org/Appropedia:CategoryTree, or perhaps this might be an expansion of the "Popular Tags" interactive graph that @cesswairimu created on publiclab.org/stats, but i'm really not sure! Thanks for reading 😃 At a later time, i look forward to thinking about this.

[jywarren]

Hi @keshavsethi - I saw this error in the log and it seemed related to spam2:

ERROR["test_ban_and_unban_authors_in_spam2", #<Minitest::Reporters::Suite:0x00007f5080091758 @name="SpamTest">, 163.256268584]
 test_ban_and_unban_authors_in_spam2#SpamTest (163.26s)
Minitest::UnexpectedError:         Capybara::ExpectationNotMet: expected to find css "a[href='/ban/7'" but there were no matches
            test/system/spam2_test.rb:51:in `block in <class:SpamTest>'

[Tlazypanda]

@jywarren Just a heads up created the fto here #8055 ✌️

[jywarren]

Thank you!

[keshavsethi]

Hi @keshavsethi - I saw this error in the log and it seemed related to spam2:

ERROR["test_ban_and_unban_authors_in_spam2", #<Minitest::Reporters::Suite:0x00007f5080091758 @name="SpamTest">, 163.256268584]
 test_ban_and_unban_authors_in_spam2#SpamTest (163.26s)
Minitest::UnexpectedError:         Capybara::ExpectationNotMet: expected to find css "a[href='/ban/7'" but there were no matches
            test/system/spam2_test.rb:51:in `block in <class:SpamTest>'

@jywarren I have made some changes in system tests of spam2 i.e banning if users, publish and spam(these three tests were giving some errors). Now it is working fine, please review #8034 Thanks!!

[ebarry]

Thank you @keshavsethi @Tlazypanda @jywarren all so much for work on this! Exciting! 🌈

[jywarren]

OK, final change: note how in lg (large) width, the final tab text disappears to make space for the Sort by dropdown button, which should allow this to work smoothly at all widths.

Note I had thought of adding an icon to Sort by so it could become more compact too, but the filter icon is not very readable in my opinion:

https://fontawesome.com/v4.7.0/icon/filter

So I opted to leave the text.

[jywarren]

And here it is on the contributors tab view:

[ebarry]

Great, so just to make sure i understand, lg or Large view is what is shown on phones or other mobile devices?

The contributors tab looks great!

[jywarren]

Great, so just to make sure i understand, lg or Large view is what is shown on phones or other mobile devices?

It's all based on screen width breakpoints -- https://getbootstrap.com/docs/4.4/layout/overview/#responsive-breakpoints -- so, large devices are those with:

// Large devices (desktops, 992px and up)

I chose this because it's when the tabs start to hit the button on the right side, and tested it at all widths. Thanks!

[jywarren]

[jywarren]

OK, so solution will be to just hide the counts on the narrowest screens (like many phones):

Done in 831aea1 !

[jywarren]

Can be checked momentarily at https://stable.publiclab.org/tag/balloon-mapping