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

Disable reply buttons for guests. Fixes #326 #451

Merged
merged 3 commits into from
Dec 1, 2014
Merged

Disable reply buttons for guests. Fixes #326 #451

merged 3 commits into from
Dec 1, 2014

Conversation

jerone
Copy link
Contributor

@jerone jerone commented Nov 26, 2014

Fixes #326

This will disable all reply buttons in the forum and script issues pages for guests that are not logged in.
A tooltip will be shown on hover and on click, saying that user needs to login to post.
The reply textbox (which shows up when scrolling down) is also removed when not logged in.

@jerone jerone added PR READY This is used to indicate that a pull request (PR) is ready for evaluation. UI Pertains inclusively to the User Interface. bug You've guessed it... this means a bug is reported. labels Nov 26, 2014
@Martii
Copy link
Member

Martii commented Nov 26, 2014

Suggestion... try at least these urls logged out and look for green buttons (this list could grow):

... It would be super cool to have some symmetry across the board with that not just replies (avoid vote/flag/remove though... that can be done later after we stop keeping sizzle from remerging/modifying his stuff and my keeping him busy with the production issues) ... the little popup is very kewl. :)

@jerone
Copy link
Contributor Author

jerone commented Nov 26, 2014

Suggestion... try at least these urls logged out and look for green buttons ...

What do you mean by this?

It would be super cool to have some symmetry across the board with that not just replies

This was going to be my next PR.

@Martii
Copy link
Member

Martii commented Nov 26, 2014

You sort of answered your own question here by telling me it's your next PR... basically that PR should be present before this gets merged in order to show that symmetry is happening... I'll be supporting this whichever methodology you choose from above... although it would be nice to have a QSP to take it back to where a user was including any sub QSP e.g. if a user is on a QSP'd item the login page redirects back to that QSP'd URI... something USO didn't do.

@jerone
Copy link
Contributor Author

jerone commented Nov 26, 2014

What does QSP mean?

@Martii
Copy link
Member

Martii commented Nov 26, 2014

My apologies... been using this acronym terminology on USO since around 2005 and definitely before that.

http://www.wikipedia.org/wiki/Query_string

QS === Query String
QSP === Query String Parameter

…into issue-326

Conflicts:
	views/pages/scriptIssuePage.html
@Martii
Copy link
Member

Martii commented Dec 1, 2014

Tooltips at http://localhost:8080/users/Marti/comments don't appear to be styled the same as http://localhost:8080/discuss/Test_1

{{^authedUser}}
<script>
$(function () {
$('.btn-comment-reply').parent().tooltip().click(function(){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function(){

Should be function () {

@Martii
Copy link
Member

Martii commented Dec 1, 2014

Head style diff shows:

<link href="/redist/npm/bootstrap-markdown/css/bootstrap-markdown.min.css" media="all" type="text/css" rel="stylesheet">

... present on http://localhost:8080/discuss/Test_1 but not on http://localhost:8080/users/Marti/comments

@jerone
Copy link
Contributor Author

jerone commented Dec 1, 2014

@Martii commented on 1 dec. 2014 21:54 CET:

Tooltips at http://localhost:8080/users/Marti/comments don't appear to be styled the same as http://localhost:8080/discuss/Test_1

Actually reply buttons shouldn't be there at all. There's even no logic attached to it. Please open another issue for this.

@Martii
Copy link
Member

Martii commented Dec 1, 2014

There's even no logic attached to it.

I know... I think Zren/sizzle thought maybe having a redirect to login/discussion and come back to it... so it's up to you what you want to do.

@jerone
Copy link
Contributor Author

jerone commented Dec 1, 2014

@Martii commented on 1 dec. 2014 22:10 CET:

There's even no logic attached to it.

I know... I think Zren/sizzle thought maybe having a redirect to login/discussion and come back to it... so it's up to you what you want to do.

I don't think its useful to start a conversation on that page. A link back to that discussion is sufficient. To me it makes more sense that that button should be hidden.

Edited.

@Martii
Copy link
Member

Martii commented Dec 1, 2014

I don't its useful to start a conversation on that page

Concur.

To me it makes more sense that that button should be hidden.

Concur... so what do you want to do with this issue/pr then?... mitigation, or new issue, or fix it since it's probably small fix and is partially related to the disabling the reply button for guests.

@jerone
Copy link
Contributor Author

jerone commented Dec 1, 2014

@Martii commented on 1 dec. 2014 22:23 CET:

To me it makes more sense that that button should be hidden.

Concur... so what do you want to do with this issue then?... mitigation, or new issue, or fix it since it's probably small fix and is partially related to the disabling the reply button.

New issue. It's a small style issue, but it requires logic in the controllers to hide the button only on that page, as includes/comment.html is a shared file.

@Martii
Copy link
Member

Martii commented Dec 1, 2014

Mitigation is me... new issue is you please... they more or less mean the same thing but from a different aspect... other than that +1 here and...

New refs

Martii added a commit that referenced this pull request Dec 1, 2014
Disable reply buttons for guests. Fixes #326

merge
@Martii Martii merged commit 6138676 into OpenUserJS:master Dec 1, 2014
@jerone jerone deleted the issue-326 branch December 1, 2014 22:02
@Martii Martii added needs mitigation Needs additional followup. and removed needs mitigation Needs additional followup. labels Jan 13, 2015
@Martii Martii removed the PR READY This is used to indicate that a pull request (PR) is ready for evaluation. label Feb 7, 2015
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug You've guessed it... this means a bug is reported. UI Pertains inclusively to the User Interface.
Development

Successfully merging this pull request may close these issues.

Guests can see reply button in forum, resulting in 404 after posting
2 participants