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?