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

fix: fix the size of the image in CarouselCard component #431

Merged
merged 6 commits into from
Dec 11, 2018

Conversation

cmcartaya
Copy link
Collaborator

fix: #408

@coveralls
Copy link

coveralls commented Nov 29, 2018

Pull Request Test Coverage Report for Build 442

  • 13 of 13 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 77.284%

Totals Coverage Status
Change from base Build 416: 0.2%
Covered Lines: 1243
Relevant Lines: 1539

💛 - Coveralls

const ref = {};
expect(carouselCardContainerStyles(ref)).toBeNull();
});
it('should return an object with a heght of 100% if the parentNode the ref passed has an height set', () => {

Choose a reason for hiding this comment

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

height

height: '100%',
});
});
it('should return an object with a heght of 340 if the parentNode of the ref passed does not have an height set', () => {

Choose a reason for hiding this comment

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

same ^^

@@ -5,7 +5,7 @@ import classnames from 'classnames';
import { Provider } from './context';
import Indicators from './indicators';
import AnimationButton from './animationButton';
import { getItemIndex, getChildTabNodes, insertChildOrderly } from './utils';
import { getItemIndex, getChildTabNodes, insertChildOrderly, carouselCardContainerStyles } from './utils';
Copy link
Member

@LeandroTorresSicilia LeandroTorresSicilia Dec 5, 2018

Choose a reason for hiding this comment

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

getContainerStyles since it is a function, would be carouselCardContainerStyles if it were a const.

<div>
<img className="rainbow-carousel-image_image" src={src} alt={assistiveText} />
<div className="rainbow-carousel-image_content-image-container">
<div className="rainbow-carousel-image_image" style={this.getImageSrc()} alt={assistiveText} />

Choose a reason for hiding this comment

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

the alt attribute is only for img element so here we need to use the AssistiveText component in order to screen readers take the assistive text. Then:

<div className="rainbow-carousel-image_content-image-container">
    <div className="rainbow-carousel-image_image" style={this.getImageSrc()} />
    <AssistiveText text={assistiveText} />
</div>

@LeandroTorresSicilia LeandroTorresSicilia merged commit 4c10fac into master Dec 11, 2018
@TahimiLeonBravo TahimiLeonBravo deleted the fix-img-size-carousel-card branch September 23, 2019 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: fix the size of the image in CarouselCard component
3 participants