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

added default aria-label to colorpicker input #1788

Merged
merged 16 commits into from
Jul 18, 2018

Conversation

blackbaud-conorwright
Copy link
Contributor

Added a default aria-label to colorpicker components for when one is not supplied

Resolves: #1381

@codecov-io
Copy link

codecov-io commented Jul 2, 2018

Codecov Report

Merging #1788 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1788      +/-   ##
==========================================
+ Coverage   99.98%   99.98%   +<.01%     
==========================================
  Files         410      410              
  Lines        8585     8589       +4     
  Branches     1267     1269       +2     
==========================================
+ Hits         8584     8588       +4     
  Misses          1        1
Impacted Files Coverage Δ
...modules/colorpicker/colorpicker-input.directive.ts 99% <100%> (+0.04%) ⬆️

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 da7d277...1b55b25. Read the comment docs.

Copy link
Member

@Blackbaud-SteveBrush Blackbaud-SteveBrush left a comment

Choose a reason for hiding this comment

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

A few small things and I think this one is good to go.

@@ -28,6 +28,7 @@ import {
} from './types';

import { Subscription } from 'rxjs/Subscription';
import { SkyResourcesService } from '../resources';

Choose a reason for hiding this comment

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

import {
  SkyResourcesService
} from '../resources';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -87,7 +88,8 @@ export class SkyColorpickerInputDirective
constructor(
private elementRef: ElementRef,
private renderer: Renderer,
private service: SkyColorpickerService
private service: SkyColorpickerService,
private skyResourceSvc: SkyResourcesService

Choose a reason for hiding this comment

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

Use the keyword 'Service' to be consistent. While we're in here, we should also rename the service parameter (okay since it's private).

private colorpickerService: SkyColorpickerService,
private resourceService: SkyResourcesService

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@Blackbaud-SteveBrush Blackbaud-SteveBrush changed the title added default aria-label to colorpicker input [HOLD] added default aria-label to colorpicker input Jul 3, 2018
@blackbaud-conorwright blackbaud-conorwright changed the title [HOLD] added default aria-label to colorpicker input added default aria-label to colorpicker input Jul 16, 2018
@blackbaud-conorwright
Copy link
Contributor Author

no longer on hold. The input field is now readonly and will not be hidden

@Blackbaud-SteveBrush Blackbaud-SteveBrush self-assigned this Jul 17, 2018
@Blackbaud-SteveBrush Blackbaud-SteveBrush merged commit 67feaf7 into master Jul 18, 2018
@Blackbaud-SteveBrush Blackbaud-SteveBrush deleted the fix-colorpicker-input-label branch July 18, 2018 12:26
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