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

fix: announce ‘no match’ information on subscription filter #2584

Merged
merged 4 commits into from
Nov 14, 2022
Merged

fix: announce ‘no match’ information on subscription filter #2584

merged 4 commits into from
Nov 14, 2022

Conversation

wanghoppe
Copy link
Member

@wanghoppe wanghoppe commented Oct 31, 2022 •
edited by azure-boards bot
Loading

fix: AB#758

@codecov
Copy link

codecov bot commented Oct 31, 2022 •
edited
Loading

Codecov Report

Merging #2584 (8d676a4) into master (95e7c38) will decrease coverage by 0.00%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2584      +/-   ##
==========================================
- Coverage   64.99%   64.99%   -0.01%     
==========================================
  Files         915      915              
  Lines       26368    26379      +11     
  Branches     5192     5197       +5     
==========================================
+ Hits        17137    17144       +7     
- Misses       9231     9235       +4
Impacted Files Coverage Δ
src/@batch-flask/ui/select/select.component.ts 81.33% <ø> (ø)
...elect/select-dropdown/select-dropdown.component.ts 91.89% <66.66%> (-4.94%) ⬇️

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 95e7c38...8d676a4. Read the comment docs.

gingi
gingi reviewed Oct 31, 2022
View reviewed changes
Copy link
Member

@gingi gingi left a comment

Choose a reason for hiding this comment

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

Looks great! Great use of the LiveAnnouncer utility. Only suggestion is to localize the strings being announced.

src/@batch-flask/ui/select/select-dropdown/select-dropdown.component.ts Show resolved Hide resolved
@wanghoppe wanghoppe enabled auto-merge (squash) November 1, 2022 06:01
gingi
gingi previously approved these changes Nov 1, 2022
View reviewed changes
@wanghoppe
Copy link
Member Author

@gingi I made some changes to the unit test to pass the CI check, you might need to approve it again.

@wanghoppe wanghoppe merged commit 7ab8668 into master Nov 14, 2022
@wanghoppe wanghoppe deleted the hoppe/accessibility10 branch November 14, 2022 02:13
@cRui861
Copy link
Member

cRui861 commented Feb 1, 2023 •
edited
Loading

Fixed #2634

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gingi gingi gingi approved these changes

@cRui861 cRui861 Awaiting requested review from cRui861 cRui861 is a code owner

@dpwatrous dpwatrous Awaiting requested review from dpwatrous dpwatrous is a code owner

@zfengms zfengms Awaiting requested review from zfengms zfengms is a code owner

@paterasMSFT paterasMSFT Awaiting requested review from paterasMSFT

@seth-barshay seth-barshay Awaiting requested review from seth-barshay

@wiboris wiboris Awaiting requested review from wiboris wiboris is a code owner

@skapur12 skapur12 Awaiting requested review from skapur12 skapur12 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@wanghoppe @cRui861 @gingi