Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI: Vocab & Bootstrap? #103

Closed
30 of 32 tasks
Zren opened this issue May 16, 2014 · 72 comments
Closed
30 of 32 tasks

UI: Vocab & Bootstrap? #103

Zren opened this issue May 16, 2014 · 72 comments
Labels
enhancement Something we do have implemented already but needs improvement upon to the best of knowledge. UI Pertains inclusively to the User Interface.
Milestone

Comments

@Zren
Copy link
Contributor

Zren commented May 16, 2014

Edit:

Currently redoing the UI with Bootstrap & FontAwesome & whatever is needed.

Demo: http://nameless-hollows-5487.herokuapp.com

Progress

Discussion Category

  • CategoryListPage
  • AllDiscussionListPage
  • CategoryDiscussionListPage

Discussion

  • DiscussionPage
  • NewDiscussionPage

Script

  • ScriptListPage / Homepage
  • ScriptPage
  • ScriptEditMetadataPage
  • ScriptEditScourcePage
  • ScriptViewSourcePage

Script Issue (Extends Discussion)

  • ScriptIssueListPage
  • ScriptIssuePage
  • NewIssuePage

Script Group

  • GroupListPage
  • GroupScriptListPage

Library (Extends Script)

  • misc

User

  • UserPage
  • UserScriptListPage
  • UserCommentListPage
  • UserEditMetadataPage
  • UserEditPreferencesPage
  • UserNewScriptPage
  • UserWriteScriptPage
  • UserManageGithubSyncedScriptsPage
  • UserListPage

Auth

  • Register (& Login as the username input was removed)

Moderation

  • Graveyard
  • Flagged

Admin

  • UserListPage
  • UserPage
  • ApiPage

Misc

  • 404 Page

Original Post:

Seeing as most users are probably coming from Userscript.org, I'm wondering why you changed some of the vocabulary.

  • Tags => Groups
  • Forums => Garage (Ehhh...)
  • Script Requests => Beggars Corner (okay this is a snarky as hell title for it)

Comments on specific routes.

/

  • Pagination only shows the current page with directional buttons. This combined with the minimal amount of groups (summing to ~10 scripts) make the library seem tiny.
  • Giant menu icons.

/scripts/

  • Any element in the description that's not a p or table has mini text. This is because of a body { font-size: 62.5%; } ...

Demos

Looking at the UI made me want to redo the entire thing. When checking out the tracker I also saw #77, and thought if you're going to implement it for a 3rd parties, you might as well use it for the site as well. This made me think of http://datatables.net/. I noticed that NodeJS + Mongoose has a package for it on the server side https://www.npmjs.org/package/mongoose-datatable. Now I haven't checked into SEO with DataTables, so I'll have to look further into it. Anyways, I made a quick demo with it and Bootstrap 3.

http://codepen.io/Shadeness/pen/zBomq

I also tried cloning Userscript.orgs frontpage (without DataTables this time).

http://codepen.io/Shadeness/pen/LmyFB

Essentially, I wanted to point out that Bootstrap + FontAwesome would help make the site look less like it came from the 90s and more like it's a copy-paste site from the 2010.

@jerone
Copy link
Contributor

jerone commented May 16, 2014

+1 on all your points.

I know that some of the naming have reference to Greasemonkey.

I would also suggest changing the naming of "Issues" for scripts to "Discussions". This sounds more positive & would also allow discussions on that script besides issues.

http://codepen.io/Shadeness/pen/LmyFB

Really liking your second example!

I'm no designer, but haven't really found the time and effort to list all HCI issues with this design.
We also need to know that this is still work in progress!

@moshmage
Copy link

TBH: I don't like the design Userscripts had. It was lacking information and at the end of it all it just felt like someone droped a Word table on it and moved on with it.

I agree, thought, that the website feels like it came from the 90's [it made me feel a mix of feelings, but I also got the same urge as you did to redo the thing] but after some time looking at it I now think that instead of a "redo" of the whole thing the, the thing that it really needs is some minor adjustments to paddings, margins and the likes (and "maybe" a change in font).

I do agree with you on the Icon bar; Had I read the issues list before I'd have put #104 in here instead.

I don't agree with your view on the naming of the section and kind'a like the humor behind it (LOL @ The Beggars Corner), but that's just personal and wouldn't really matter if it changes. Though the thing about "groups" vs "tags" ... meh. I got more annoyed at the fact that one script can't have two groups than the wording of said thing.

Plus, we have to remind ourselves that the website is months old, if that much, so let'em deal with the backend first so we (i'm assuming you're a frontend dude aswell) can shine'it afterwards ;)

@Martii
Copy link
Member

Martii commented May 16, 2014

I got more annoyed at the fact that one script can't have two groups than the wording of said thing.

Give it some time. I'd like to see more than one group as well per script but we need more moderation tools available first.

remind ourselves that the website is months old

Yes. Improvements will come at a speedier rate eventually.

@sizzlemctwizzle
Copy link
Member

@Zren

Tags => Groups

I hate tags. Script groups have different behavior.

Forums => Garage (Ehhh...)

There are no forums (seriously I have a future feature up my sleeve that would make the use of "forum" term incorrect, which is why I use the generic "discussion"). The Garage is just a discussion category.

Pagination only shows the current page with directional buttons.

Completely intentional. I don't see the point of having a long list of page numbers.

Giant menu icons.

Yup. I'd like to shrink them.

This made me think of http://datatables.net/.

Looks nice. I'll give it some thought.

@jerone

I would also suggest changing the naming of "Issues" for scripts to "Discussions".

-1 I really don't want to do that. It's an issue tracker (you can open and close them, and eventually I'll add labels), so I'd be strange to give it a generic name.

@moshmage

I got more annoyed at the fact that one script can't have two groups than the wording of said thing.

Incorrect, although I need to add text letting people know how this feature works. You can only create one group for a script. You can add your script to as many existing groups as you desire. This is why I chose the name "group" and not "tag". I'm trying to prevent people from creating many unnecessary and redundant groups. Basically choose the groups you create carefully, because you have a limit. So yes, this is intentional behavior and not a limitation of the implementation (in fact I had to do a lot of work to accomplish this).

@Zren
Copy link
Contributor Author

Zren commented May 17, 2014

@sizzlemctwizzle

There are no forums (seriously I have a future feature up my sleeve that would make the use of "forum" term incorrect, which is why I use the generic "discussion"). The Garage is just a discussion category.

Garage isn't a synonym of discussion, the only way a new user would know it was for discussion was if they randomly clicked it. It would be fine if you were already at the "Forums" index, and the "Garage" was a subforum, but this is a nav link in the header of every page on the site.

@moshmage

I don't agree with your view on the naming of the section and kind'a like the humor behind it (LOL @ The Beggars Corner), but that's just personal and wouldn't really matter if it changes. Though the thing about "groups" vs "tags" ... meh. I got more annoyed at the fact that one script can't have two groups than the wording of said thing.

I didn't know you couldn't add multiple "groups". Group is probably the better term atm then.

@Zren
Copy link
Contributor Author

Zren commented May 19, 2014

I just noticed there's a general discussion here: https://openuserjs.org/discuss which would probably be a better main nav link.


I also started implementing this UI here: https://github.com/Zren/OpenUserJS.org/tree/ui

I've done the Script List Page and the Script About Page.

You can view it using this live demo on heroku. It's using the default dev db (I'll try to clone it if that's not ok).

Here's some screenshots if it crashes: https://github.com/Zren/OpenUserJS.org/wiki/Screenshots

I had to change the pagination/sort/search technique to use the query string. Well I didn't have to, but you shouldn't be storing optional variables like that in the route/path anyways. I'll be adding another flag variable to the query string filtering libraries later (which is why they aren't filtered out at the moment).

Some things I found out while coding this so far:
We really need an id/slug field in the Script schema. Getting which script a route belongs to is cumbersome (having to generate the script.installUrl before querying the db). And generating urls is painful as well.

@sizzlemctwizzle
Copy link
Member

@Zren

Garage isn't a synonym of discussion

Yeah I realized awhile ago that I need a better name, but we already had the icon for it by then so I was stuck.

It would be fine if you were already at the "Forums" index, and the "Garage" was a subforum, but this is a nav link in the header of every page on the site.

I'm against using "Forums" but I do want to rename that discussion category to something that implies discussing user script development. "Forum" won't work because of the feature I mentioned earlier. Since Greasy Fork already has a forum and seems committed to it, I guess I can divulge my idea. Basically discussions are currently comment-based (there's a topic), but what I'm going to do is that when two or more registered users are on a discussion page at the same time it will transform into a chatroom (hence "discussion" covers both comment and chat-based communication). When the number of users on that page drops below two, it'll revert to comment-based communication and the chat history will be preserved on the page (probably as a scrolling comment box). I hope I explained my idea well enough. If you want me clarify, please don't hesitate to ask.

I just noticed there's a general discussion here: https://openuserjs.org/discuss which would probably be a better main nav link.

I didn't link to the general discussion area because I want to discourage off-topic discussion on the site.

It's using the default dev db (I'll try to clone it if that's not ok).

I'd prefer if the dev db isn't used for anything world-facing.

but you shouldn't be storing optional variables like that in the route/path anyways.

Yeah, moving the pagination/sort/search access to the query string is probably better. I guess I just really love using routes and got a little route-crazy.

We really need an id/slug field in the Script schema.

It already does (script._id), but it is only used internally.

Getting which script a route belongs to is cumbersome (having to generate the script.installUrl before querying the db). And generating urls is painful as well.

I prefer using this because of its benefits (we can identify a script based on its contents: @name and @namespace) even if it is more cumbersome than generating a meaningless id. I don't find it painful since there are functions that help do this work for you. It never bothered me. Also it allows us to have meaningful urls, and even if we switched we'd have to keep this, so we'd just be making this more complex not simpler. I might be willing to use a hash of username, @name, and @namespace (if it's present) in places where we don't need a readable url.

As for your demo, nice work. I do like the changes overall but it still looks a bit bare and boring. Can we at least get some icons in there? It appears that the login/register is unimplemented. What were you planning on doing with that? I'd like to keep the one-input login myself, but I don't know how that would fit with the redesign. And we decided long ago to use blue and grey for the color scheme. Also, I'm going to reiterate: they're not tags, they're groups. From your previous comment it appears you agree, but you use "Tags" in your demo.

Anyway, I'd really appreciate some opinions on the site design from others. I don't enjoy making all the decisions alone.
@OpenUserJs/admin @OpenUserJs/backend @OpenUserJs/frontend

@Zren
Copy link
Contributor Author

Zren commented May 19, 2014

I'd prefer if the dev db isn't used for anything world-facing

I'll get on that then.

they're not tags, they're groups.

Where didn't I change i- oh in the "tag cloud". Sorry.

we decided long ago to use blue and grey for the color scheme

Ah. I just grabbed a theme off http://bootswatch.com/ that looked most like US.org. Was going to change the colour scheme after doing all the layouts anyways. Good to know.

get some icons in there

You want more icons? Hehehehehehe. http://fontawesome.io/icons/ Basically anything off that can be used. I'll try not to make it overkill.

Re: ids/slugs

You have 3-4 piece of data right now to act as a primary key.

scriptType (scripts/libs) + scriptAuthor + scriptNamespaceSlug (optional?) +  scriptNameSlug

The fact that we're duplicating and accepting routes without the scriptNamespaceSlug tells you it's pointless and you should remove it as script namespaces are ugly sluggified urls and bloat up the whole url. You don't even need it unless a user has two scripts with the same name scriptNameSlug. And in most cases, the namespace will be the same for all the user's scripts, making it redundant.

Same with scriptType, it's unnecessary without needing to distinguish two scripts by the same user with the same slug.

So you really only need: userName and scriptNameSlug.

@sizzlemctwizzle
Copy link
Member

Ah. I just grabbed a theme off http://bootswatch.com/ that looked most like US.org.

I see. Personally, I like this one: http://bootswatch.com/superhero/ I always advocated a darker background, but other people disagreed. But then again, I don't pretend to know much (if anything) about UI.

So you really only need: userName and scriptNameSlug.

That makes sense to me. Until people started using the site I didn't realize that the namespace portion turned out to be a hot mess with no real meaning. I'm for getting rid of it (although it'll require a fair amount of work since it's used to store scripts in S3 as well). I kind of like dividing user script and library routes (I like including the information in the url), but I'll admit the code would be a lot simpler and cleaner if it was eliminated.

@Martii
Copy link
Member

Martii commented May 19, 2014

I'd really appreciate some opinions on the site design from others

I assume you mean publicly. I'm still getting the feel (including logical layout which in some circles is better than the other sites :) of the site so I can dig into the source a little deeper. If you haven't noticed most of my scripts and even GM is improving upon existing structures. Some of the technologies used I have never heard of before but luckily I'm usually a quick learner and appreciate every linkage. The site could use a little improvement with CSS on cell phones to maintain consistency across pages, on desktops the font scaling is really unusual with the percentage size... some of my lists are bigger than others and the markdown syntax is identical minus content, and of course my announced issues with some of the plumbing. As far as recommendations of certain technologies I usually write things from scratch when it comes to web design. Reinventing the wheel I suppose. I'm moving as fast as I can to catch up... money and other duties are also a factor for me right now. :) I can help you make some of the decisions but only within my current expertise. Just remember not to get overwhelmed... sometimes that is easy to do especially under the current crunch on USO.

EDIT: You might consider weekly or monthly "get togethers" like Moz does to help ease the decision making processes and not try to have everyone do it in one shot... and a well established release schedule with the exception of emergency releases and of course some delays.

@jerone
Copy link
Contributor

jerone commented May 19, 2014

I also started implementing this UI here: https://github.com/Zren/OpenUserJS.org/tree/ui

I've done the Script List Page and the Script About Page.

You can view it using this live demo on heroku.

Just some fast feedback...

I love this new design. It looks more modern and professional. A lot of sites use this kind of design, which is intuitive and easy to look at. Now it also shows more information, which requires less scrolling. The current design, to my taste, is a bit out-dated and childish, I do however like the font.

I do agree that it misses some icons. Specially at the header, but this could be fixed by adding a logo. Another suggestion is to show the script's @icon in the listing when available (and default image when not).

The color might be a bit to related to USO and I would like to see it in the agreed upon (where?) blue and grey color scheme.

The detail page is a little crowded to my taste. It's not clear that the 3 links (About, Source Code, Issues) are part of a tabbar.
I would move the information below the script name (e.g. By jerone — Last updated 3 months ago — Installed 0 times.) also on the right side. That would make room for more information from the script (like @license, etc).
Above suggestion distinguish the custom script description more from the standard site layout.
Always add an submit button after a search field.

@sizzlemctwizzle
Copy link
Member

@Zren I intend to use your proposed design. Do you need some help from me to adapt it to the entire site, or is that something you've got handled? I ask because I see you've been doing a lot of work on your branch and I don't want to get in your way. If you need anything from me, just ask.

@Zren
Copy link
Contributor Author

Zren commented May 24, 2014

I renamed my ui branch to dev as it was starting to get full of other thing. I remade the ui with code for an eventual PR.

I wrote a script to grab ~600 scripts from userscript.org to populate my local db.

I did the ScriptEditMetadataPage the other day. Added :username onto the route to stay consistent with the routes (yes we can pull the username from the session user).

image

I added a helper function called app_route for routing which can be seen here, It works similar to Express v4. Eg: app_route('/').VERB(callback).VERB(callback);.

I also found out you can use: ? like so :varname? in routes to declare an optional route param. This should cleanup a bunch of duplicate route definitions. However /script/:username?/:namespace?/:scriptname/edit does NOT match /script/scriptNamespaceSlug/scriptNameSlug. Found out the hard way.

Edit: Moved checklist to OP.

@sizzlemctwizzle Could you implement the ScriptEditScourcePage and ScriptViewSourcePage? I haven't setup the fake AWS script yet, so that'd be helpful.

@Zren
Copy link
Contributor Author

Zren commented May 25, 2014

Moved the checklist to the OP.

I finished the UserProfile + UserEditPage today (I broke each up into 2 pages).

image

@sizzlemctwizzle
Copy link
Member

Good work Zren. Look great!

Could you implement the ScriptEditScourcePage and ScriptViewSourcePage?

Yeah, I'll get on that but I'm certain that it won't look too pretty. JS & HTML are my thing. I don't really know what I'm doing with CSS (and I know the editor requires some).

@sizzlemctwizzle
Copy link
Member

@Zren

Brilliant work! Love what you've done with discussions. I can tell you're going for a "US.o if it had been created in this century" feel. Keep up the great work. You're kicking ass and taking names.

Edit: I'm still trying to figure out your new layout for the templates so I can implement those two pages.

@Zren
Copy link
Contributor Author

Zren commented May 26, 2014

Thanks. I styled the discussions after http://try.discourse.org/


Here's a template page to get you started.

  • That'll include the top and bottom navs.
  • I opted to keep the head and body tags out of the header/footer as it makes pages more versatile.
  • "Root" templates (the page) go in views/pages/* and anything you include goes in view/includes/*. Mustache imports are relative to the views/ folder, not relative to the template file itself. So you can use {{> includes/blarg.html }} even in view/pages/blargPage.html.

For the controller, here's an example.

  • I used optionser.authedUser = req.auth.user instead of options.username = req.session.user.name. This is so I can get the user.userPageUrl and possible other things like user.messagesCount in the future.
  • I used async parallel pattern for all the new controllers as most have more than one query. This also allows us to concat common tasks between controllers.

And why not, one more:
http://nameless-hollows-5487.herokuapp.com/register

Yes I know you can't login on my herokuapp (it's due to the oauth callbacks not allowing the host). The login works though on localhost.

@sizzlemctwizzle
Copy link
Member

If that's the template, is there any way we can abstract that code away (I'm think mostly of the controller) so we don't have to repeat ourselves?

The login/register looks nice. How about a modal popup? Like normally login shows, but when you click register it switches.

@Zren
Copy link
Contributor Author

Zren commented May 26, 2014

The top one is bare bones as it's just defining stuff and metadata changes on each page. The second one could probably have the pagination abstracted. The sort/search could as well probably, though we need to keep the query = Model.find() visible in the controller.

@sizzlemctwizzle
Copy link
Member

The second one could probably have the pagination abstracted. The sort/search could as well probably

Couldn't you adapt the modelsList library (specifically the listModels function) to do this? Is there a reason you decided to stop using it, rather than extending/improving it? Or did you just adapt/replace it somewhere else that I'm not seeing?

@Zren
Copy link
Contributor Author

Zren commented May 26, 2014

Couldn't you adapt the modelsList library (specifically the listModels function) to do this? Is there a reason you decided to stop using it, rather than extending/improving it?

Keep legacy code working without breaking everything at once, and modelList is designed around route params. I opted to break up parsing the individual objects into modelParser as well.

@sizzlemctwizzle
Copy link
Member

and modelList is designed around route params.

Actually, you can pass it an object as well, although that was never really used anywhere so it might not work too great.

I opted to break up parsing the individual objects into modelParser as well.

Ah, I see. That makes sense and looks better. Even while I wrote the code, I knew the structure of my modules where haphazard (see #72). I'm glad someone can bring some order.

@Zren
Copy link
Contributor Author

Zren commented May 27, 2014

Did a little refactoring. I simplified pagination a bit, and removed the need to define which model we're using in modelQuery.parseModelListSort as we can grab the model from query.model.

Eg: Zren/OpenUserJS.org@5d1dbca...42d42da#diff-9ded95676cd9a25c6466192ba063401aR20

Going to redo Issues next.

@Dexmaster
Copy link

"Internal Server Error" Just saying ...

On 28 May 2014 02:23, Chris Holland notifications@github.com wrote:

Did a little refactoring. I simplified pagination a bit, and removed the
need to define which model we're using in modelQuery.parseModelListSortas we can grab the model from
query.model.

Eg: Zren/OpenUserJS.org@5d1dbca...5b4aa8f#diff-9ded95676cd9a25c6466192ba063401aR20Zren/OpenUserJS.org@5d1dbca...5b4aa8f#diff-9ded95676cd9a25c6466192ba063401aR20

Going to redo Issues next.


Reply to this email directly or view it on GitHubhttps://github.com//issues/103#issuecomment-44348072
.

Best Regards, Andrii M.

mob. tel: +38 097 583 58 35
e-mail: dexteritymaster@gmail.com
skype: Dexmaster

@sizzlemctwizzle
Copy link
Member

@Dexmaster #112

@Zren
Copy link
Contributor Author

Zren commented May 28, 2014

@Zren
Copy link
Contributor Author

Zren commented Jun 4, 2014

On headings, change the header link icon class from fa fa-paragraph to fa fa-link to have a similar icon to what GitHub has. Would turn it to this:

Done, and shrinked the icon to 66%.

what are the requirements for mobile? Because it doesn't work on multiple mobile browsers I've tested the demo.

Looks like I needed to add <meta name="viewport" content="width=device-width, initial-scale=1.0"> to get the phone view on high DPI devices.

How far are you from submitting a pull request? Is there anything I can do?

What's left is:

  • Tweak all the pages broken by the change in theme + adding the gutter bg.
  • Libraries
  • 404
  • Fix GitHub sync.
  • Pull upstream/master

I've manage to get my own github client id/key so I can now login on the demo. I also port forwarded my localhost fasks3 for the herokuapp so it'll save scripts now. Which means I can actually tackle the github sync issue (hopefully). I'll try and see if I can finish it tonight, and get a PR ready.

I'll be adding the following optional Environment Variables btw:

  • AUTH_CALLBACK_BASE_URL
  • DEV_AWS_URL

@cletusc
Copy link
Contributor

cletusc commented Jun 4, 2014

Looks like I needed to add...

Take a look at some of the practices in HTML5 Boilerplate--that's one of them. I've adapted quite a bit of it within my own personal projects, scrapping a few bits for Bootstrap and other things. In short, meta that is well recognized (mobile-first and so-on), 1-2 scripts in the head (modernizer), and all the other JS at the bottom as needed. 😄

@jerone
Copy link
Contributor

jerone commented Jun 5, 2014

Great work, have been following your progress behind the scenes!

Uhg. Need sleep.

http://nameless-hollows-5487.herokuapp.com/?library=true

The install button on a library page isn't necessary.

@sizzlemctwizzle
Copy link
Member

Does /graveyard and /flagged work?

Yes, but they don't have support for comments yet and the graveyard isn't finished (it should let moderators undo removal).

@Martii
Copy link
Member

Martii commented Jun 6, 2014

Is flagged where the threshold is reached and then it becomes flagged? ... because I don't really see any other option to "flag" something.

@sizzlemctwizzle
Copy link
Member

Is flagged where the threshold is reached and then it becomes flagged?

Yes and then it can be removed by moderators.

because I don't really see any other option to "flag" something.

You're an admin. You can't flag. You just remove.

@Martii I'm just going to remove this script and user. I've read enough of the obfuscated code to know that it tries to send a report user request on FB. This is not what it says that code does in the comments so it goes against the "doesn't do what is advertised" rule. It is intentionally misleading.

This user is clearly a scammer. Their scripts send requests to "like" several predefined profiles (or in some cases submit a friend request), and promise the user that they will receive likes on their profile if they install the script. This is totally misleading and a scam. I'm going to remove this user as well. I just don't have patience for stuff like this.

@Martii
Copy link
Member

Martii commented Jun 6, 2014

remove

I kind of figured that but didn't want to overstep until the TOS is established some more in #116

You can't flag.

Interesting... there may come a point when this is needed due to time constraints or general "not sure"... but if it's based off of the rating exclusively I suppose that could work... the "undo" option might be a good thing too instead. Thanks.

@sizzlemctwizzle
Copy link
Member

there may come a point when this is needed due to time constraints or general "not sure"...

I think maybe someday we'll add the ability to have a moderator discussion on a script for those tricky cases (which would be visible to all moderators and admins on some sort of dashboard thing).

but if it's based off of the rating exclusively I suppose that could work...

No, there is real flagging for non-admins (however an upvote does count against a flag). But since admins are basically not a part of that user moderation system (users flagging and moderators removing once the threshold has been met), I don't think they have a right to flag something. Plus there's the potential that admins could be targeted which is why they can't be flagged (moderators just have a higher threshold). Admins can see the number of flags on something (non-admins can't so we don't cultivate a hivemind) to help identify something that might need reviewing. I imagine the site one day having a ton of moderators (since we don't have to put to much trust in them) and a few admins.

the "undo" option might be a good thing too instead.

Yeah that's a big deal in making the system work. If someone screws up, we can always fix it (right now it's completely manual).

@Zren
Copy link
Contributor Author

Zren commented Jun 6, 2014

"Load Scripts" is a VERY unoptimized route. It basically hits every folder of every single repo you own (that's not a fork). Not supporting importing from forks is silly as well. Anyways, that's a sure fire way to eat up our Rate Limit allotment (5000 / hr for each of GitHub's api categories). Since I've no idea what the output of that is (as it takes forever and breaks when it does output), I'm not quite sure what it looked like.

It looks like that route was mostly useless anyways, as it has nothing to do with syncing. Syncing hadn't been working for me as I just wasn't uploading the script first.

NewScriptPage

Note: Import (URL) doesn't work.

GithubRepoPage

Import script from Github Repo & List instructions for synchronizing.

Now with checking blob.size!

To complete this page I'm thinking of adding an optional /.../upload?url= route for the "Import" buttons. That'll take a bit of work. I'm going to leave out the GitHub specific pages for the ui PR so I can mess around with them more.

@Martii
Copy link
Member

Martii commented Jun 6, 2014

Firstly, the script must already be uploaded to the site.

This isn't currently true... the import process automatically posts to the site... is this behavior changing?

@Zren
Copy link
Contributor Author

Zren commented Jun 6, 2014

This isn't currently true... the import process automatically posts to the site... is this behavior changing?

Import = ?

Pushing to a repo with a webhook set? This currently does not add new scripts (found through testing on oujs.org). It will update existing scripts however.

Or is that the List Scripts buttons that does hundreds of requests and takes a minute and doesn't work?

Edit:

Should note that I did these tests on https://openuserjs.org/

http://i.imgur.com/1BPBVX3.png

https://github.com/ZrenTest/TestUserScript/blob/master/TestUserJS.user.js

https://openuserjs.org/users/zrentest

Edit:

So it does work, on ZrenTest at least. Zren probably has way too many directories in all my repos and times out on the request maybe. The github api actually paginates the request for all my repos (which I don't think we handle?). I also discovered https://github.com/mikedeboer/node-github which might help.

When fetching a repo tree, you can optionally set the request to be recursive: https://developer.github.com/v3/git/trees/#get-a-tree-recursively

It works on ZrenTest though. http://i.imgur.com/t4IbEMy.png and now that I've seen the form, I should be able to implement importing.

@sizzlemctwizzle
Copy link
Member

@Zren
Imo the best way to improve the import feature is to just automatically list their GitHub repos (this is a single GH api call, or a few in the rare case of pagination) and let users choose which ones we should scan for scripts. This should probably be done via some client-side JavaScript (interfacing with our server via JSON api), or initiated by the click of a button, so they don't have to wait for this to load to use the other methods to add scripts.

Not supporting importing from forks is silly as well.

I did it partly to lower the number of requests, and also because I want people using the site to fork. I don't know, maybe we should provide a link on imported scripts back to GitHub.

It looks like that route was mostly useless anyways, as it has nothing to do with syncing. Syncing hadn't been working for me as I just wasn't uploading the script first.

Just because it doesn't work for you doesn't mean it hasn't worked for others (including myself). I didn't test it on an account that had a ton of repos. I see you got it working on a test account.

This currently does not add new scripts

Intended behavior. You need to explicitly tell us what scripts you want us to publish via importing first.

I also discovered https://github.com/mikedeboer/node-github which might help.

This provides a ridiculous number of features that we don't need (or will probably ever need). Dealing with the GH api is pretty simple and this would be overkill for what we're doing (we already have plenty of dependencies already). Edit: Meh go ahead and use it if it really makes things easier.

The github api actually paginates the request for all my repos (which I don't think we handle?)

Not yet but this could be better fixed by using a JSONP library.

When fetching a repo tree, you can optionally set the request to be recursive:

That might kill us memory-wise. I think it's better to load many small JSON parsed objects into memory asynchronously than a few huge ones. It's the reason I import the script data by using the raw link instead of using the JSON api (it can be massive when loaded into memory).

To complete this page I'm thinking of adding an optional /.../upload?url= route for the "Import" buttons.

I'm not crazy about this. Basically it makes it super simple for users to add a script that isn't theirs. I really don't see a good reason to add this feature. If we do, we must link back to where the script was imported from.

@Zren
Copy link
Contributor Author

Zren commented Jun 9, 2014

client-side JavaScript

Would have to have a seperate github oauth id/key for that.

Edit: Meh go ahead and use it if it really makes things easier.

yay.

/.../upload?url= ... I'm not crazy about this.

Not going to do it then as I'm not sure how to apply limits while uploading anyways. Least we can check the github blob size before hand.


POST http://nameless-hollows-5487.herokuapp.com/users/Zren/github/import
Form Data:
    user: Zren
    repo: OpenUserJS.org
    path: libs/markdown.js

Noticed that GitHub import didn't support importing JS libraries.

Initial code for the redone githubImport.

  • Lose ability to import all scripts in a repo at once. The user could middle click the import button to remain on the import page. Import is currently a POST request, but we could change it to a GET request and use an to leave the repo list behind. We could also make the import button submit an ajax request, and show a link to the new script page on success instead of redirecting there.
  • Add ability to import javascript libraries.
  • Paginate repo List.
  • We are NOT currently paginating a repo's blob tree.
  • Repo page does filter out files that are not JS.
  • Indicate when a file is too large to import.
  • List a github user's repo list on one page, and list the repo's file tree on another.

@sizzlemctwizzle
Copy link
Member

Would have to have a seperate github oauth id/key for that.

I meant the client-side JavaScript would interact with the server-side via a JSON api.

We could also make the import button submit an ajax request

This is probably the best option, but POST works for now (could you use target="_blank" or the equivalent for forms? Is there one?).

Add ability to import javascript libaries.

So you're solution for the no name problem is to just use the file name minus the ".js"? I guess that works since I can use spaces in filenames on GitHub.

Repo page does filter out files that are not JS.

Meh, not important imo. They are supposed to know which repos they want to import scripts from.

One more thing: I don't see the "Edit Meta" link on my scripts anymore. Did you move this somewhere? Also on the 404: it's "you're" not "your". Oh, and the "Add Webhook" link here is broken.

Anyway, very nice work. Much more usable than what we had. It loads noticeably faster and is simple to use.

@Zren
Copy link
Contributor Author

Zren commented Jun 9, 2014

The install button on a library page isn't necessary.

Is now labeled as Raw Source.

I don't see the "Edit Meta" link on my scripts anymore.

Oops. Typo'd when changing a check for options.isYou to options.authorTools, in case we change the controllers to allow moderators to view those links in the future.


  • Table heading are now links to default sort operations (they don't inverse).
  • Added moderation + admin landing pages. Linking ?flagged=true for scripts/users/libraries, the graveyard, and the old flagged page for moderators. Linking the api keys page and the old user list page for admins.

Edit:

  • Moved /users/Zren/github/import to /users/Zren/github/repo. Made /users/Zren/github/import accept GET requests. Check if a github repo has >1 script files, and if so, open in a new tab.

@Martii
Copy link
Member

Martii commented Jun 9, 2014

The install button on a library page isn't necessary.

Is now labeled as Raw Source.

Raw Source implies zero formatting including any configured headers... are you sure this is what the labeling should be? It was quite confusing when I visited briefly one of the other sites and definitely not as intuitive as a plain old "Install" (the libraries, as pointed out earlier, are a special case for not directly installing usually). As I've been with GH since the early days I personally am quite familiar with this nomenclature however not everyone is especially if they aren't an author. Usually shorter strings are easier to manage too along with potential "word play". (think installWith) More things to think about. :)


and if so, open in a new tab.

Do 100% of all browsers now have tabs 100% of the time?


The user could middle click the import button to remain on the import page.

How does one "middle click" on a cell phone?

@Zren
Copy link
Contributor Author

Zren commented Jun 9, 2014

I was following what the upstream branch was calling it that button.

  1. Remove "Install" button on libraryPage.
  2. Move "Raw Source" button on libraryPage to libraryViewSourcePage.
  3. Leave as it origionally was labeled as "Install" even on libraryPage.
  4. Rename it to "Raw Source" only on libraryPage.
  5. ?

@Martii
Copy link
Member

Martii commented Jun 9, 2014

The site revamps you are doing look great... I'm just trying to leverage some of the learning curve for others. We all have our skill sets but sometimes changes are needed for compromise so we don't leave the inexperienced in the dust. :) (btw more questions that should be asked above)


On listings of multiple scripts where there is a version, add a "v" in front of it, like so:

This could be a nightmare with translators and takes up screen real estate... best to just leave the versioning as is. I grew up with v1.0.0 but soon realized that it wasn't as good of an idea as I was led to believe. A user.js would be more of a suitable place for this for those who want it.


but POST works for now

Yah I'm getting both anxious and excited for the remodel of the UI to be nearing live... both the original and Zrens are nice but we don't need a complete remodel of every plumbing aspect right off the bat... "open open open open" :) ;)

@Zren
Copy link
Contributor Author

Zren commented Jun 9, 2014

Do 100% of all browsers now have tabs 100% of the time? / Phones

I changed it from a POST request (form data send in the body of the request) to a GET request (query string parameters in the url). I'm using <a target="_blank"> (now) which works is IE6 (creates a new window). It works on my Nexus7 (chrome) as well. If it doesn't support tabs, it'll just open like a regular link.

On listings of multiple scripts where there is a version, add a "v" in front of it, like so:

I'm opting not to do this as some scripts have v1.2.3 as their version, and vv1.2.3 looks weird.

@Zren
Copy link
Contributor Author

Zren commented Jun 9, 2014

@sizzlemctwizzle

That might kill us memory-wise. I think it's better to load many small JSON parsed objects into memory asynchronously than a few huge ones. It's the reason I import the script data by using the raw link instead of using the JSON api (it can be massive when loaded into memory).

Does nodejitsu have a low mem req?

@Zren
Copy link
Contributor Author

Zren commented Jun 9, 2014

I think I'll keep the old checkbox UI under github/ and link to github/repos from it, but only list at most the 10 5 3 most recently updated repos. That should keep if from taking too long and tearing up our rate limit with tree requests.

Edit:

Done: http://nameless-hollows-5487.herokuapp.com/users/Zren/github

ToDo:

  • expand async.render(tasks, function(){preRender(); render }); to async.render(tasks, function(err){ if (err) return next(); res.render(...); }) to follow style guideline.

Other than that, I'm going to freeze this for a PR.

@sizzlemctwizzle
Copy link
Member

We have 2 drones (seperate instances of our app) that have 256MB of RAM
each.
On Jun 9, 2014 4:21 AM, "Chris Holland" notifications@github.com wrote:

@sizzlemctwizzle https://github.com/sizzlemctwizzle

That might kill us memory-wise. I think it's better to load many small
JSON parsed objects into memory asynchronously than a few huge ones. It's
the reason I import the script data by using the raw link instead of using
the JSON api (it can be massive when loaded into memory).

Does nodejitsu have a low mem req?


Reply to this email directly or view it on GitHub
#103 (comment)
.

@sizzlemctwizzle
Copy link
Member

Closing this because of the merging of #129. If there's anything left, create new issues.

@Zren Zren closed this as completed Jun 14, 2014
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Something we do have implemented already but needs improvement upon to the best of knowledge. UI Pertains inclusively to the User Interface.
Development

No branches or pull requests

7 participants