Ignore on Mobile Feature Development Questions


(Hank G) #1

After spending many hours getting up to speed and dabbling to the point of having the feature working in a side branch I’m ready to implement the “ignore on mobile” feature highlighted in Github Issue #7840 for real. I have a couple of best practices questions that I couldn’t divine, or missed, in the documentation on best practices etc. So before getting started I thought I’d ask them here. Before I get too deep the changes I’d be making are relatively minimal (most of my time was in getting up to speed on Rails and on the code base):

  • BlocksController.create: Add a redirect return so the display refreshes after creation
  • PeopleController.show: Add a generation of a @block field for use in the HAML code (similar to how done in privacy settings and in the main UI’s JavaScript code, but the JavaScript does it browser-side)
  • show.mobile.haml: add the new control and additional logic for the ignore/block so similar to main UI, including new label for new button
  • Update locales with new string for new label

So with that background, here are my questions or places for confirmation:

I can make the calculation of the block object either in the HAML or in the controller. This is the data structure that should exist if the current user is blocking the person whose data is being viewed by the show method. When reading up on Rails it said a best practice was to minimize the amount of code in the view and push that to the controller. That’s pretty standard MVC best practices. So it makes sense to me to do that. However since no one has ever needed the block object before I wanted to double check that this is the place people would want me to place it.

With respect to generating the block I found some utility code in apps/models/user/querying.rb (User::Querying) and thought I’d use that except the comment at the top sounds pretty dire “this file should not exist…”. The actual call is one line using find_by on the blocks data structure on the Person object so I coded it directly instead of using it. Should I instead be using the User::Querying module?

On the view itself, the desktop UI has an icon with a tool tip. Should I be trying to create an icon button with a label or have it emulate the bootstrap button we use elsewhere with no icon but just the label?

With respect to the labels I’m going to use the locale labels not hard coded strings. First question, I assume it’s better to reuse strings rather than create new ones with their own hierarchy unnecessarily. For example on the confirmation dialog box I intended to use the “are you sure” string rather than create a new one “Ignore this user?”. I still need to create a string label for the button though.

When I read the “Contribute translations” page on fixing found hardcoded strings it says to add the new key to “en.yml/devise.en.yml/javascript.en.yml”. Does that mean I add it to any of those or all of those? I’d be inclined to add it to at least en.yml and devise.en.yml and leave it off javascript.en.yml since this is just in the Ruby code. I don’t speak any languages fluently so I wouldn’t be able to contribute a translation to any other language either, so I’m assuming it will default to one it finds in English if that’s all that’s defined. Is that correct?


(Hank G) #2

I’ve done an implementation based on my above reasonable assumptions and answered the question on the translation of missing strings in other languages. I still need to make sure I edited all of the appropriate locale strings.

I’m also dealing with Jasmine scripts showing up with errors both on develop and master. When I run against my code there are no errors in the code I’m editing but I’d rather have all my tests passing before I submit this PR and the a few others I have for some other front end items.


(Hank G) #3

I’m going to call jasmine:ci running all green as good enough to start initiating pull requests. I can’t get all greens in FireFox or Chrome right now…


(Dennis Schubert) #4

Please only touch the English locale files. All changes you make to other locales will get lost. These changes have to be made on webtranslateit.


(Hank G) #5

Understood. Fortunately I had no other language ability so wouldn’t try to do another language. However do I just edit the first file, the first two files, or all three files? It runs fine with just the one file edited so that’s all I’ve done so far.


(Dennis Schubert) #6

That depends? en.yml contains generic language strings required by the backend and its templates, devise.en.yml contains strings used by the authentication framework, and javascript.en.yml contains strings passed into the frontend to be used by JS. If you need translations for JS work, add them to javascript.en.yml. If you need translations inside the backend, add them to en.yml