Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(Status): fix order of applied props #961

Merged
merged 4 commits into from
Feb 25, 2019
Merged

Conversation

layershifter
Copy link
Member

Fixes #958.

@codecov
Copy link

codecov bot commented Feb 25, 2019

Codecov Report

Merging #961 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #961      +/-   ##
==========================================
+ Coverage    80.7%   80.72%   +0.01%     
==========================================
  Files         659      659              
  Lines        8459     8465       +6     
  Branches     1431     1495      +64     
==========================================
+ Hits         6827     6833       +6     
  Misses       1618     1618              
  Partials       14       14
Impacted Files Coverage Δ
packages/react/src/components/Status/Status.tsx 100% <ø> (ø) ⬆️
...ages/react/test/specs/commonTests/isConformant.tsx 93.36% <100%> (+0.19%) ⬆️

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 45e8790...9653e35. Read the comment docs.

expect(element.prop('role')).toBe(role)
})

test('attributes passed by consumer has more weight', () => {
Copy link
Contributor

@kuzhelov kuzhelov Feb 25, 2019

Choose a reason for hiding this comment

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

nit-nit: maybe we could just state supported scenario, like client's attributes override the ones provided by Stardust

test('attributes passed by consumer has more weight', () => {
const wrapper = mount(
<Component
{...{ ...requiredProps, [IS_FOCUSABLE_ATTRIBUTE]: false }}
Copy link
Contributor

Choose a reason for hiding this comment

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

would suggest to make it a bit more readable:

const clientAttributes = {
   [IS_FOCUSABLE_ATTRIBUTE]: false,
   accessibility: noopBehavior
}

const wrapper = mount(<Component {...requiredProps} {...clientAttributes} />
...

expect(element.prop('role')).toBe(role)
})

test('attributes passed by consumer has more weight', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 thanks for adding this one!

Copy link
Contributor

@kuzhelov kuzhelov left a comment

Choose a reason for hiding this comment

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

would like @jurokapsiar to take a look on these changes, just to be absolutely sure this strategy won't get into conflict with existing accessibility behaviours

@layershifter layershifter merged commit fd7da6a into master Feb 25, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix/status-fix-order branch February 25, 2019 20:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🧰 fix Introduces fix for broken behavior. 🚀 ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants