-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat(card): support left and right image placement #1981
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
Conversation
…and img-width props
…reat on mobile. FWIW, left/right aren't very good for mobile anyway unless it is a very small image and text.
Codecov Report
@@ Coverage Diff @@
## dev #1981 +/- ##
==========================================
- Coverage 64.68% 64.64% -0.05%
==========================================
Files 154 154
Lines 2911 2919 +8
Branches 801 804 +3
==========================================
+ Hits 1883 1887 +4
- Misses 739 743 +4
Partials 289 289
Continue to review full report at Codecov.
|
…onals for pushing the image onto the childNodes stack.
This is failing in CI because CircleCI is reporting issues connecting to NPM and isn't able to build correctly. It seems that NPM is to blame in this case. Can I please have someone review this and merge if acceptable? Thank you in advance. |
@tmorehouse @pi0 or someone who has CI access: can you please trigger a build (or tell me what you prefer to do) so that this will run clean now that NPM has corrected connectivity issues? |
class: { 'img-fluid': props.fluid }, | ||
attrs: { src: props.src, alt: props.alt } | ||
class: {'img-fluid': props.fluid}, | ||
attrs: {src: props.src, alt: props.alt, height: props.height, width: props.width} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this always add a width
and height
attributes to the rendered element?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is null it is omitted
You can trigger a rebuild by making a minor edit (such as editing a comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merged dev and pushed in passing build
class: { 'img-fluid': props.fluid }, | ||
attrs: { src: props.src, alt: props.alt } | ||
class: {'img-fluid': props.fluid}, | ||
attrs: {src: props.src, alt: props.alt, height: props.height, width: props.width} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is null it is omitted
src/components/card/card-img.js
Outdated
if (props.left) { | ||
staticClass += '-left' | ||
} | ||
if (props.right) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these be a series of if { } else if { } else if { }...
statements rather than a series of if { }
statements?
This would set an order of precedence for the position props, i.e. maybe top
, bottom
left
then right
. This would then prevent the strings from being possibly concatenated (i.e. card-img-left-right-bottom
)
And maybe, should we use start
and end
instead of left
and right
?
src/components/card/card.js
Outdated
} | ||
if (props.imgRight) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these if
statements also be if { } else if { }...
to handle order of precedence of the position props?
* dev: (31 commits) feat(card): support left and right image placement (bootstrap-vue#1981) fix(collapse/toggle): "collapsed" class cleared when component updated (bootstrap-vue#2102) fix(form-file): Add validation of single file (bootstrap-vue#2028) chore(docs): minor update to the b-form-input docs chore(docs): Minor update to b-form-input docs feat(table): Add row-unhovered event (bootstrap-vue#1874) feat(table): Support contextmenu event binding for table rows (bootstrap-vue#2064) docs(table): fix minor typo (bootstrap-vue#2093) feat(table): Support sorting on nested object properties (bootstrap-vue#1868) perf(modal): optimize model.resetScrollbar, resolves bootstrap-vue#1831 (bootstrap-vue#1837) docs: Update images reference section (bootstrap-vue#1999) fix(observe-dom): fix comment typo (bootstrap-vue#2084) chore(modal): trivial word fix in comment (bootstrap-vue#2089) (docs): Fix grammer in Intro readme (bootstrap-vue#2092) fix(modal): prevent scrolling on .modal-content focus, fixes bootstrap-vue#1748 (bootstrap-vue#2060) feat(pagination): added slots for first, prev, next, last, and ellipsis. Fixes bootstrap-vue#1870. (bootstrap-vue#1980) Handle state change on validated fields. (bootstrap-vue#1984) fix(docs): input group prepend slot typo (bootstrap-vue#2059) fix(dependencies): replace opencollective with opencollective-postintall (bootstrap-vue#2067) fix(docs): change b-input-group attribute 'left' to 'prepend' (bootstrap-vue#2017) ...
Added img-height and img-width props in case someone wants to break it themselves. Addressed #1958