CoHoop / CoHoop-Rails

CoHoop's Rails application
2 stars 0 forks source link

[MVP] Integration #15

Closed gabriel-dehan closed 11 years ago

gabriel-dehan commented 11 years ago

Minimal integration for the MVP

gabriel-dehan commented 11 years ago

@Djeezy haven't you seen that all the classes are snake cased and dash separated (users-microhoops-list) ? Why suddenly use camel case (leftSidebar) for CSS classes ?

We need to have consistency ;)

gabriel-dehan commented 11 years ago

@Djeezy https://github.com/CoHoop/CoHoop-Rails/commit/a57b35371fbc7edd73e8b03119937873ec9b8d96#app/views/feeds/show.html.erb

Why did you change partially classes case, and removed IDs ? A lot of Ids and some classes are used by javascript files to create ajax related behaviors, those features are now broken.

From now on, be extremely careful with what you do, please. I'll fix most of it, though.

(And be logical : it is a list of microhoops so the class is either microhoopsList or microhoops-list. Not listMicrohoops which is actually the imperative form : an order. ;) )

Now we are going to use snake case, because I clearly don't want to modify ALL the javascript and I want to have a consistency between all the code.

Edit2: Oh, and the < i > balise is not used anymore, we use < em > now, < i > is only used for icons in twitter bootstrap ^^

Please refer to : https://github.com/CoHoop/CoHoop-Rails/wiki/Front-end-development-conventions from now on. Feel free to add anything you'd like to be specified, you are the lead integrator, but those are the basic rules now.

gabriel-dehan commented 11 years ago

1/ https://github.com/CoHoop/CoHoop-Rails/blob/a57b35371fbc7edd73e8b03119937873ec9b8d96/app/views/feeds/_microhoop.html.erb

It is the microhoop view, the class should be prepended with microhoop- not feed- ;)

2/ In layout.css.scss I see : width: ((($wrapper / 12) * 3) - ($padding * 2)) - 2px; // 2px coz of well block border

Wouldn't you be better off using the grid system ? As it actually do exactly what you are trying to do here : complexe calculation taking in account the wrapper and a padding (gutters) Plus, as we are using SASS, there is no reason for not using it : it doesn't add any horrible markup

Yoksy commented 11 years ago

Well, sorry for the inconvenience. First, I'm used to camel case all the time, thats something I won't do for CoHoop alpha first "design" (lol), I get it. Secondly, I've throw away a lot of ID, coz too many ID are just wrong (I really want to make the design modular). But I didn't think about javascript, sorry too. Well, now I won't delete any classes and won't change your javascript :p

But to be clear : as for the next design, I will change some front-end conventions, coz we will be back from scratch : camel cases, minimals ID (as I'll take care of all client-side javascript, except for etherpad related stuff I think). If some classes or ID are required by the application, it just suxxx :-x

For the tags, I was a mistake coz of bootstrap ^^

About layout : that was a fucking mistake too, I shall just spank myself with a rose whip xD

gabriel-dehan commented 11 years ago

As for the front-end conventions when we will start over : We won't be starting totally over, most of the correct code base that already exists will be used, I will just throw away patterns we don't need and tight coupled code. Furthermore, "client-side" javascript (as for ajax request handling) in Rails is tightly coupled with server side behavior, as you use javascript-erb files that often make calls to helpers or even models. Plus the javascript actually directly respond to rails controllers, those are javascript views, with pure ruby inside; Which would actually require you to be really familiar with the back-end development. It would be far harder for you to take care of it than you think, you would need to know the design and the rails quite well. But part of the javascript (part which does not handle request) is actually in the assets and this will be your job to write. But you will obviously have to cope with the javascript I will already have written "server-side".

IDs are required by the application, sometimes it is normal, because we need features to be unique as we know we won't have it multiple time in a page, and that if it is the case the javascript should not be triggered multiple times.

I think that for some parts, IDs are pretty important as DOM access through the IDs are thousand times faster.

I don't see the problem with some classes or IDs required by the application, when you have an application with javascript IDs and classes are tightly coupled with it as the javascript works on the DOM. And the server side actually generates those classes and IDs, so everything is tightly coupled, in my opinion. Some IDs are not even generated manualy but are automaticaly added by Ruby On Rails, and that is just fine. It is the case everywhere : it does not suck it is natural. And as ruby on rails actually automaticaly add snake case ids, I think it is just logical to keep up with this and keep the snake case, don't you think ?

And FFS, stop with the "'coz" just use plain english already, slang and abbreviated words are no good in a technical discussion. If you were to write a dialogue in a book, okay, but that is just not the case.

PS : You might say that you are used to camel case but you still wrote some snake-case classes, I checked the commits, and that is what was weird : no consistency ;)