Semantic-Org / Semantic-UI-Angular

Semantic UI Angular Integrations
MIT License
557 stars 117 forks source link

Semantic UI Angular Revamp #32

Closed ClickerMonkey closed 8 years ago

ClickerMonkey commented 9 years ago

Looking to revamp the codebase with a nearly complete library.

ClickerMonkey commented 9 years ago

I realize the way I organized my code & examples is atypical, if anyone thinks a specific file structure is better please suggest it so I can reorganize it. OR this request can just be accepted and someone can help out :-P

jlukic commented 9 years ago

@ClickerMonkey :clap :clap: These are tremendous strides towards an official Semantic UI integration for Angular. Massive kudos to Phil for all of his hard work here.

@Semantic-Org/angular Please when you guys get some time, can you come take a peek around here? Your feedback is super important, and I'm eager to hear.

He's also linked me to the documentation he's created for this which is a good place to poke around.

caitp commented 9 years ago

i'll take a quick look

caitp commented 9 years ago

I'm not a fan of the actual code contents of the library, or the way it's organized. I don't think it would be good to throw out @m0t0r and other's work for it. It does look like a suitable sister-library, but could really use some re-org too assist in generating smaller builds

ClickerMonkey commented 9 years ago

Could you expand on that a little? What don't you like about the code?

Not liking it's organization is understandable, I have extremely limited experience in structuring JS libraries. If you point me to a structure, I can mend it.

I don't agree with the idea of not accepting it because other's work will have to be thrown out - the only merit in that line of thinking is preserving ego - consistent & complete code is more important. (I don't mean to come off strong/pushy here, but there's no other way to phrase what I'm thinking).

By "could really use some re-org too assist in generating smaller builds" do you mean creating separate downloads for each individual element/module/etc like Semantic?

Thanks for taking the time in doing this and being so thorough.

caitp commented 9 years ago

Could you expand on that a little? What don't you like about the code?

The heavy reliance on Semantic-Org's own JS (and by extension proper jQuery) --- it saves a lot of work, and is definitely a valid project in its own right, but I don't agree that it belongs in this particular repository.

Not liking it's organization is understandable, I have extremely limited experience in structuring JS libraries. If you point me to a structure, I can mend it.

There are a few things --- it's pretty weird to just send a completely different project as a pull request against another project, so the process is questionable. It's not fixing anything, and is certainly breaking things. It's just not how you'd do this. Contributions ought to be evolutionary steps forward. The code style is pretty hard on the eyes, but I'll leave that to personal preference --- The fact that it's so large, has such large dependencies, and genuinely doesn't really need to, is the biggest problem for me. Part of this could be mitigated with a modular build system (similar to jQuery or lodash), but that would take some effort to set up.

I don't agree with the idea of not accepting it because other's work will have to be thrown out - the only merit in that line of thinking is preserving ego - consistent & complete code is more important. (I don't mean to come off strong/pushy here, but there's no other way to phrase what I'm thinking).

This is covered above --- This isn't really fixing anything, and is most certainly breaking other things.

Thanks for taking the time in doing this and being so thorough.

Not a problem, thanks for contributing to the semantic project. @jlukic and @m0t0r's points of view matter a lot here, so just because I'm not a fan of it doesn't mean it stops in its tracks. Even if the PR is not accepted, I believe it's perfectly reasonable for it to live as a separate project in the org, with slightly diffferent goals

ClickerMonkey commented 9 years ago

The heavy reliance on Semantic-Org's own JS

This is very purposeful, I didn't want to duplicate the logic that already existed in SemanticUI. I wanted the directives to behave exactly like they would in SemanticUI. I think I'm understanding now that the current code base (this project) is basically a replacement for the SemanticUI JS - correct? If so, I imagine that's why the current code base lacks any sophisticated SemanticUI modules - it's not easy to redo everything in angular (although it's definitely easier to do it in angular than plain JS IMO).

I wanted it to be easy for people who know SemanticUI to pick up the library since it's very similar.

(just an explanation as to why I did it that way)

There are a few things --- it's pretty weird to just send a completely different project as a pull request against another project, so the process is questionable. It's not fixing anything, and is certainly breaking things. It's just not how you'd do this.

I was instructed to do this from @jlukic (unless I misunderstood his advice to me).

The code style is pretty hard on the eyes

Ha, to each their own. I think the same thing about the current code base :-P

Thanks again.

jlukic commented 9 years ago

There are a few things --- it's pretty weird to just send a completely different project as a pull request against another project, so the process is questionable. It's not fixing anything, and is certainly breaking things. It's just not how you'd do this.

I was instructed to do this from @jlukic (unless I misunderstood his advice to me).

This is true, my advice was to look for a way to merge the two projects. I think we can all agree that there hasn't been much progress in the Angular integrations over the last few months, as the pulse shows.

My hope is that we can find a way to move forward with a singular solution (warts and all) that would be, at the very least, up to date with the mainline project.

I'm not an angular developer so I have no way to evaluate the code, but after spending some time organizing the Ember and React integrations I've realized that it will be unlikely that someone could re-implement all of SUI's APIs without relying on its javascript modules. The project just moves too fast to be constantly re-implemented across all channels.

@caitp Do you know how this was done with UI Bootstrap. Am I out of line for thinking that it at least somewhat relies on the original js.

richstandbrook commented 9 years ago

Just want to add my long silent voice—I'm a beginner to middling Angular developer (it's been evolving so fast it's hard to reach experienced!). I have a number of applications built with Angular and Bootstrap UI and was really excited about Semantic UI, I'm trying to use it more but the lack of Angular implementation makes it much less convenient. Massive kudos to @ClickerMonkey for implementing Angular bindings, I was thrilled to see it—although I hadn't realised it was reliant on jQuery—this is a serious consideration as it would add quite a massive weight to the distribution, I would much prefer native Angular.

That all said, I don't have a massive amount of time but I really want to see this project move forward. If there is someone with the time and experience I might be compelled to fund some development time, perhaps we could crowd source some funding?

caitp commented 9 years ago

@jlukic, it is true that it takes more effort to build them on top of angular alone, and i think it's worth doing this version built on top of the jQuery module. But, web developers are often on budgets in terms of the # of bytes they can use, in spite of caching and timing, and other strategies for improving load times. So to cater to these, a pure angular approach is what i was aiming for when i wrote the initial set of bindings.

The best way to get people interested in working on it, is probably to improve the marketing and notoriety of semantic itself. Ui-bootstrap and angular-strap exist and are fully formed because bootstrap is ubiquitous, and is used for prototyping sites all over the place. People hack on it because they need it for their own projects.

Anyways, i do think it's a good start, and i can do a more thorough review at the start of September. I think it's worth having it, but i don't think it should replace the pure angular codebase, rather live side by side it, same repo or not. This isn't an ego-driven point of view, it comes from my experience as a web developer working for many different shops, and from my experience wIth angular's users

ClickerMonkey commented 9 years ago

I think the core decision that needs to be made is whether "Semantic-UI-Angular" is a library that complements Semantic UI or a library that complements Angular. Using jQuery+SemanticUI JS makes it possible with the current community size to keep up with the rapid progress Semantic UI makes. Not only that but there are a lot of useful things Semantic UI does (ex: logging & performance). I agree - it could be possible to keep this codebase purely Angular if we had a larger community - but there's no guarantee when that may happen. Perhaps having a complete library that relies on Semantic UI JS may draw more people towards Semantic UI. The progress on the current binding has been lacking as @jlukic mentioned - its a lot of work to completely redo everything that's been done with Semantic UI in angular AND keep up with it's development.

Keeping the two bindings separate makes sense - how would we name the respective projects so it's clear why they're different?

Random thought - has anyone thought about/looked into what jQuery functions Semantic UI uses vs what angular's "jqLite" provides?

ClickerMonkey commented 9 years ago

That last remark was a crazy thought of detecting whether jQuery is present - and if it's not creating a fake jQuery which uses angular.element() and fills in additional methods Semantic UI requires.

caitp commented 9 years ago

angular.element is jQuery if jQuery is loaded before angular, no need to reinvent that stuff.

ClickerMonkey commented 9 years ago

Yes, but an improvement to what I've done would be to use something that extends angular.element for Semantic UI JS code - to remove the requirement of jQuery. If jQuery is present you're right - but I wonder how hard it would be to get Semantic UI to work without jQuery.

caitp commented 9 years ago

The version bundled with angular is pretty limited, mostly limited to class api and very basic dom read/write, so my bet is pretty difficult without being very careful

jlukic commented 9 years ago

I think the best solution for the time being is to provide both options side by side, and be explicit that this is a repo specifically to continue integrating Semantic UI components without jQuery in a pure angular way. This way those who want to contribute to angular bindings, can decide whether they want jquery or not.

I think perhaps Semantic-UI-Angular-JQuery could work as a name, what do you think?

I plan on doing the same thing with the React contributors who are having the exact same issue now.

Overall, I don't think it will be too fragmenting to have two 'official' integrations.

I'm a bit busy today (speaking at BrooklynJS tonight) but can look at doing this tomorrow. My only concern is that we are clear on messaging so people know why there are two repos.