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.

Sign up for GitHub

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

Jump to bottom

Use new Ban icon to indicate unauthorized users #3487

Merged
merged 2 commits into from
Jan 17, 2020
Merged

Use new Ban icon to indicate unauthorized users #3487

merged 2 commits into from
Jan 17, 2020

Conversation

donker
Copy link
Contributor

@donker donker commented Jan 15, 2020 •
edited by david-poindexter
Loading

Resolves #3114

@david-poindexter
Copy link
Contributor

@donker I edited your comment on this PR to ensure it will auto close the referenced issue for which this addresses. For further reference, take a look at this link for more information on the syntax requirements. Thanks. :)

david-poindexter
david-poindexter approved these changes Jan 15, 2020
View reviewed changes
Copy link
Contributor

@david-poindexter david-poindexter left a comment

Choose a reason for hiding this comment

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

Looks good - thanks @donker

@david-poindexter david-poindexter added this to the 9.5.0 milestone Jan 15, 2020
@donker donker requested a review from valadas January 15, 2020 16:40
@valadas
Copy link
Contributor

valadas commented Jan 15, 2020

Why not use cross_out.svg ? which is already in there and looks the same?

@sleupold
Copy link
Contributor

sleupold commented Jan 15, 2020 •
edited
Loading

I would prefer an even more specific icon - e.g. greyed user with an exclamation mark or
https://www.bing.com/images/search?view=detailV2&id=FD75D92B86382515F941F6326E39008003DC09DA&thid=OIP.BEA_W8YOL3xfWPSkmBr4xQAAAA&mediaurl=http%3A%2F%2Fbritatom.gov.in%2Fimages%2Ficons%2Fpending.jpg&exph=200&expw=200&q=icon+status+pending&selectedindex=7&qpvt=icon+status+pending&ajaxhist=0&vt=0&eim=1,6&ccid=BEA%2FW8YO&simid=608015670299921645&pivotparams=insightsToken%3Dccid_gFzS8jaL*mid_DC9FE3CF08ACEB11655FD85C0BABDAE564BE5562*simid_607997773141445391*thid_OIP.gFzS8jaLhnjvbBz2UmwFVwAAAA&iss=VSI

or

https://www.bing.com/images/search?view=detailV2&id=DC9FE3CF08ACEB11655FD85C0BABDAE564BE5562&thid=OIP.gFzS8jaLhnjvbBz2UmwFVwAAAA&mediaurl=https%3A%2F%2Fstatic.thenounproject.com%2Fpng%2F43795-200.png&exph=200&expw=200&q=icon+status+pending&selectedindex=7&qpvt=icon+status+pending&ajaxhist=0&vt=0&eim=1,6&ccid=gFzS8jaL&simid=607997773141445391&pivotparams=insightsToken%3Dccid_BEA%252FW8YO*mid_FD75D92B86382515F941F6326E39008003DC09DA*simid_608015670299921645*thid_OIP.BEA!_W8YOL3xfWPSkmBr4xQAAAA&iss=VSI

@valadas
Copy link
Contributor

valadas commented Jan 16, 2020

I don't know I think the chosen one is appropriate to indicate banned and that was what was in the issue... It's like a no entry singe, which I think makes sense to me. Exclamation means important to me or warning or some such, grayed out means disabled or some such, banned I like the no-entry one...

@donker
Copy link
Contributor Author

donker commented Jan 17, 2020

Why not use cross_out.svg ? which is already in there and looks the same?

Didn't know it was there. Why didn't you mention the existence of this icon in the issue? It would have saved me a ton of time.

@valadas
Copy link
Contributor

valadas commented Jan 17, 2020

Didn't know it was there. Why didn't you mention the existence of this icon in the issue? It would have saved me a ton of time.

Sorry :)

valadas
valadas approved these changes Jan 17, 2020
View reviewed changes
Copy link
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

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

Looks good to me

@valadas valadas merged commit 995de46 into dnnsoftware:develop Jan 17, 2020
@donker donker deleted the banicon branch March 10, 2020 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@valadas valadas valadas approved these changes

@david-poindexter david-poindexter david-poindexter approved these changes

Assignees

@donker donker

Labels
Area: AE > PersonaBar Ext > Users.Web Effort: Low Priority: Medium Status: Review Type: Enhancement
Projects
None yet
Milestone
9.5.0
Development

Successfully merging this pull request may close these issues.

Unauthorized icon is the same as the roles icon in users list
4 participants
@donker @david-poindexter @valadas @sleupold