Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

Issue buttons visual improvements #20

Merged
merged 3 commits into from
Apr 30, 2015
Merged

Conversation

davidread
Copy link
Contributor

  • Edit button colours improved.
  • Abuse button made much more subtle and tooltips now work.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.22% when pulling 70804f5 on datagovuk:edit-abuse-button into b0f015f on okfn:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.22%) to 92.0% when pulling cb6f751 on datagovuk:edit-abuse-button into b0f015f on okfn:master.

@davidread
Copy link
Contributor Author

Test is now fixed

@davidread
Copy link
Contributor Author

I'm not quite sure whether this is what's needed. I've gone for the model I envisage happening in #27 however that's not the current aim. I'm not really clear how your issue.spam_count and issue.spam_state are supposed to work. Perhaps we can chat about this?

@davidread
Copy link
Contributor Author

I do want to keep the subtlness of the edit and report buttons in the PR. But I don't think the model supports the report button being a toggle, as it is not saved who reported it, so if you refresh the page then you shouldn't be able to see whether you reported it as spam or not - (or maybe you're going to store it in the session?). In which case, maybe just seeing the flash message is enough. Maybe it would be easier to just store who reported it...

@davidread
Copy link
Contributor Author

@joetsoi can you decide how to progress this PR please?

joetsoi added a commit that referenced this pull request Apr 30, 2015
Issue buttons visual improvements
@joetsoi joetsoi merged commit d262148 into ckan:master Apr 30, 2015
@joetsoi
Copy link
Contributor

joetsoi commented Apr 30, 2015

merging for now, I agree on storing the users would be better. I did think about have a json serialized list of names instead of a spam count. That would be the next better step forwards

davidread pushed a commit that referenced this pull request Jan 11, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants