Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Added inset icon for input box #266

Merged
merged 4 commits into from
May 25, 2021
Merged

Conversation

Blackbaud-AlexKingman
Copy link
Contributor

No description provided.

@blackbaud-ado
Copy link
Member

@codecov
Copy link

codecov bot commented May 24, 2021

Codecov Report

Merging #266 (c3348f1) into master (d346095) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #266   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           36        37    +1     
  Lines         1060      1072   +12     
  Branches       202       200    -2     
=========================================
+ Hits          1060      1072   +12     
Impacted Files Coverage Δ
...c/app/public/modules/input-box/input-box.module.ts 100.00% <ø> (ø)
...lic/modules/input-box/input-box-adapter.service.ts 100.00% <100.00%> (ø)
...pp/public/modules/input-box/input-box.component.ts 100.00% <100.00%> (ø)

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 5584900...c3348f1. Read the comment docs.

@Blackbaud-TrevorBurch
Copy link
Member

@Blackbaud-AlexKingman - the only question I would have (maybe for @Blackbaud-AdamFunderburk) is whether or not clicking on the icon should focus the main section of the input box? Currently nothing happens other than a border flash and the mouse icon when you hover over the icon changes to the standard icon instead of the text cursor that is displayed elsewhere on the input box.

@Blackbaud-AdamFunderburk
Copy link
Contributor

@Blackbaud-AlexKingman - the only question I would have (maybe for @Blackbaud-AdamFunderburk) is whether or not clicking on the icon should focus the main section of the input box? Currently nothing happens other than a border flash and the mouse icon when you hover over the icon changes to the standard icon instead of the text cursor that is displayed elsewhere on the input box.

Yes, it would be correct to focus the input box if the user clicks the icon.

@blackbaud-ado
Copy link
Member

@Blackbaud-TrevorBurch
Copy link
Member

@Blackbaud-AlexKingman @Blackbaud-AdamFunderburk 2 final questions:

  1. Do we anticipate that there will ever be a scenario where we have inset buttons and an inset icon? We currently do not have a visual example or test of this scenario. Should we?

  2. I'd love for @Blackbaud-AdamFunderburk to just double check the mouse icon when hovering over the icon? We now populate the focus back to the input but the mouse icon changes to a standard pointer when you hover the icon instead of the text cursor that is there on the rest of the input box. For a button this would make sense but I'd like him to verify here where the icon is mostly just visual.

@Blackbaud-AdamFunderburk
Copy link
Contributor

@Blackbaud-AlexKingman @Blackbaud-AdamFunderburk 2 final questions:

  1. Do we anticipate that there will ever be a scenario where we have inset buttons and an inset icon? We currently do not have a visual example or test of this scenario. Should we?
  2. I'd love for @Blackbaud-AdamFunderburk to just double check the mouse icon when hovering over the icon? We now populate the focus back to the input but the mouse icon changes to a standard pointer when you hover the icon instead of the text cursor that is there on the rest of the input box. For a button this would make sense but I'd like him to verify here where the icon is mostly just visual.
  1. I can't think of any scenarios right now, and I can't think of a reason to make an example without a scenario.
  2. The cursor behavior is probably fine, as long as the focus gets into the field.

@Blackbaud-AlexKingman Blackbaud-AlexKingman merged commit 63305db into master May 25, 2021
@Blackbaud-AlexKingman Blackbaud-AlexKingman deleted the input-box-inset-icon branch May 25, 2021 18:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants