Implement no-* attributes for <box>#129
Conversation
|
Ready for review |
src/TipBox.vue
Outdated
| hasBackground() { | ||
| if (this.noBackground) { | ||
| return 'no-background'; | ||
| } | ||
| }, | ||
| hasBorder() { | ||
| if (this.noBorder) { | ||
| return 'no-border'; | ||
| } |
There was a problem hiding this comment.
Probably backgroundStyle and borderStyle are more consistent names?
There was a problem hiding this comment.
It would be deceiving to say these style the backgrounds and border, since that's what customStyle does. I will rename it noBackgroundStyle and noBorderStyle because they enforce the styles of having no background or no border. Thanks for the heads up.
src/TipBox.vue
Outdated
| @@ -1,6 +1,6 @@ | |||
| <template> | |||
| <div class="alert container" :class="[boxStyle, addClass, lightStyle, seamlessStyle]" :style="customStyle"> | |||
| <div v-if="!isDefault" class="icon-wrapper" :class="[iconStyle]"> | |||
There was a problem hiding this comment.
Previously, the code renders the icon-wrapper div if the type of box is not a default box. However, I change this because the type of the box can still be the default type, and still have an icon (inserted via icon="...").
Example:
<box icon=":rocket:">
Lorem ipsum dolor sit amet,
</box>
| case 'definition': | ||
| return '<i class="fas fa-atlas"></i>'; | ||
| default: | ||
| return '<i class="fas fa-exclamation"></i>'; |
There was a problem hiding this comment.
Default boxes do not have icons.
marvinchin
left a comment
There was a problem hiding this comment.
Looks mostly good! Just a couple of suggestions, but nothing major 🙂
| return ''; | ||
| }, | ||
| noBackgroundStyle() { | ||
| if (this.noBackground) { |
There was a problem hiding this comment.
I wonder if this will make this part less verbose:
return this.noBackground? 'no-background' : '';
There was a problem hiding this comment.
I made it consistent to the coding style of other computed properties. For example,
seamlessStyle() {
if (this.isSeamless) {
return 'seamless';
}
return '';
},
fontBlack() {
if (this.light) {
return 'font-black';
}
return '';
},
365e286 to
76b8e7f
Compare
Implement positive attribute higher priority Remove redundant icon type Update code based on comments
76b8e7f to
3f63db9
Compare
|
#118 has been merged. Ready for (re)review. |
src/TipBox.vue
Outdated
| @@ -1,7 +1,7 @@ | |||
| <template> | |||
| <div class="alert box-container" :class="[boxStyle, addClass, lightStyle, seamlessStyle]" :style="customStyle"> | |||
| <div class="alert box-container" :class="[boxStyle, addClass, lightStyle, seamlessStyle, , noBackgroundStyle, noBorderStyle]" :style="customStyle"> | |||
What is the purpose of this pull request? (put "X" next to an item, remove the rest)
• [X] Enhancement to an existing feature
Fixes MarkBind/markbind#978
Requires MarkBind/markbind#1042
Requires #118 (will wait before it gets merged)
What is the rationale for this request?
Support
no-icon,no-background,no-borderattributes helps to omit the respective aspects more conveniently.What changes did you make? (Give an overview)
When no-icon is absent or icon slot is filled, the icon div will be rendered.
Because it is an or statement, the presence of
icon="..."will take precedence overno-iconWhen no-background or no-border is applied,
.no-backgroundor a.no-borderCSS class is applied respectively. Due to the position of CSS rules, it overrides styles applied by bootstrap or by ourselves. Since customs style are applied afterwards viastyle="...", it is applied last. Thus any style defined bybackground,border-color,border-left-colortake precedence overno-backgroundorno-border.Corresponding documentation in MarkBind/markbind#1042
Provide some example code that this change will affect:
You can view the syntax at https://gist.github.com/nbriannl/9369aab530e032bc93070cd8958949e3
and the corresponding output at https://nbriannl.github.io/markbind-plain-site/
Is there anything you'd like reviewers to focus on?
Proposed commit message: (wrap lines at 72 characters)
Implement no-* attributes for