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

Add Autocomplete icon #209

Merged
merged 2 commits into from
Feb 17, 2021
Merged

Add Autocomplete icon #209

merged 2 commits into from
Feb 17, 2021

Conversation

hanzei
Copy link
Collaborator

@hanzei hanzei commented Feb 9, 2021

Summary

Screenshot from 2021-02-15 09-49-48
Screenshot from 2021-02-15 09-49-34

Ticket Link

Fixes #191

@hanzei hanzei added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester 1: UX Review Requires review by a UX Designer labels Feb 9, 2021
@hanzei hanzei requested a review from abhijit-singh February 9, 2021 15:55
@hanzei hanzei added this to the v1.1.0 milestone Feb 9, 2021
@abhijit-singh
Copy link

@hanzei The icon seems to have a white background within the image which is inconsistent with the rest of the plugins. Can we replace the icon with the attached file?
ms calendar

@hanzei
Copy link
Collaborator Author

hanzei commented Feb 10, 2021

@abhijit-singh E.g. /github also uses a white background. To me there doesn't seem to be a standard. Still, I'm 0/5 on what to do there. Just wanted to check with you what the standard should be.

@abhijit-singh
Copy link

Agreed. I can see why it's almost necessary with Github, unless we solve it algorithmically, which I feel is ideal. I'll create a ticket for myself to come up with a logic that works, till then can we take a call on a case-by-case basis?

@hanzei
Copy link
Collaborator Author

hanzei commented Feb 10, 2021

@abhijit-singh Sure 👍

It seems like the file you send above (https://user-images.githubusercontent.com/14369607/107503207-ae2c5a80-6bbf-11eb-9905-a8def040f187.png) also contains a background. Could you please double check?

@abhijit-singh
Copy link

abhijit-singh commented Feb 10, 2021

@hanzei The file is transparent if you download it and check. It seems like the browser is adding that background colour for some reason.
Screenshot 2021-02-10 at 8 51 18 PM

@hanzei
Copy link
Collaborator Author

hanzei commented Feb 11, 2021

@abhijit-singh Yes, that seems to be added by my browser. Could you please provide the image as an svg? Slash command icon have to be that type.

@hanzei
Copy link
Collaborator Author

hanzei commented Feb 15, 2021

@abhijit-singh Thanks for the updated image. I've updated the screenshot. Please take another look.

@codecov-io
Copy link

Codecov Report

Merging #209 (e5a390c) into master (8437af7) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #209      +/-   ##
==========================================
- Coverage   23.25%   23.21%   -0.05%     
==========================================
  Files          64       64              
  Lines        2313     2317       +4     
==========================================
  Hits          538      538              
- Misses       1695     1699       +4     
  Partials       80       80              
Impacted Files Coverage Δ
server/command/command.go 34.78% <0.00%> (-1.59%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8437af7...e5a390c. Read the comment docs.

@hanzei hanzei requested review from iomodo and larkox February 15, 2021 14:36
@hanzei hanzei removed the 1: UX Review Requires review by a UX Designer label Feb 15, 2021
@hanzei hanzei requested a review from DHaussermann February 16, 2021 11:47
@hanzei hanzei removed the 2: Dev Review Requires review by a core committer label Feb 16, 2021
Copy link

@DHaussermann DHaussermann left a comment

Choose a reason for hiding this comment

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

Tested and passed

  • Autocomplete icon appears as expected
  • Confirmed that icon appears as expected and there is no light background visible on dark themes
  • Icon appears for all sub-commands

LGTM!

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Feb 16, 2021
@hanzei hanzei merged commit 596cad1 into master Feb 17, 2021
@hanzei hanzei deleted the 191_autocomplete-icon branch February 17, 2021 06:43
@mickmister mickmister mentioned this pull request Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add autocomplete icon
6 participants