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

Differ from flagged listings; Fixes https://github.com/OpenUserJs/OpenUserJS.org/issues/178 #179

Merged
merged 2 commits into from
Jun 18, 2014

Conversation

jerone
Copy link
Contributor

@jerone jerone commented Jun 17, 2014

Fixes #178

@Zren
Copy link
Contributor

Zren commented Jun 17, 2014

Instead of creating new booleans in the template options, why not just pass the message through?

{{^scriptList}}<tr><td>{{scriptListIsEmptyMessage}}</td></tr>{{/scriptList}}
options.scriptListIsEmptyMessage = 'No Scripts found';
if (flaggedQuery)
  options.scriptListIsEmptyMessage = 'No flagged scripts.`;

This way you can provide an ambiguous default, and refine as needed.

@jerone
Copy link
Contributor Author

jerone commented Jun 17, 2014

I've learned to never put content (HTML or text) in controllers, but apparently this is done everywhere I see.

I understand what you want, but don't see where to put this.
Problem is that scriptList.html is used in multiple pages, so I have to copy the logic to every controller. Or put the logic in applyModelListQueryDefaults, but not every option is available there (e.g. difference between libraries & scripts).
Any suggestions?

Edit:
I have the following code for the users logic in file user.js in export userListPage in function preRender:

    // Empty list
    options.userListIsEmptyMessage = 'No users.';
    if (options.isFlagged) {
      options.userListIsEmptyMessage = 'No flagged users.';
    } else if (options.searchBarValue) {
      options.userListIsEmptyMessage = 'We couldn\'t find any users by this name.';
    }

@Martii
Copy link
Member

Martii commented Jun 17, 2014

so I have to copy the logic to every controller.

I've noticed this too for another RFE I'll be posting at some point. Mine is just a need/want for a global boolean flag so mustache/html can keep "logic-less" and simple as possible. I think we could definitely use a "global" controller for all controllers inserted with require and start using some of the "publicly zoned namespace scope" a bit more (export like). In iW and CI I use the prefix of gWhatever to denote global identifiers but still within the IIFE.

@jerone
Copy link
Contributor Author

jerone commented Jun 18, 2014

So I just moved the empty lists message to the corresponding controllers. Ready for another review or merge...

@Martii
Copy link
Member

Martii commented Jun 18, 2014

+1 tested in dev. Thanks for noticing the line note and fixing this one. :) I wasn't sure if anyone would see it on the upstream merge commit or not... GH kept trying to send me to your repo with that commit hash... but I finally got it on the merge commit.

@Martii Martii mentioned this pull request Jun 18, 2014
sizzlemctwizzle added a commit that referenced this pull request Jun 18, 2014
Differ from flagged listings; Fixes #178
@sizzlemctwizzle sizzlemctwizzle merged commit cab2330 into OpenUserJS:master Jun 18, 2014
@jerone jerone deleted the issue-178 branch June 18, 2014 20:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Empty flagged lists
4 participants