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

Add content to v-progress-linear #4106

Closed
wants to merge 4 commits into from
Closed

Conversation

spezzino
Copy link
Contributor

@spezzino spezzino commented May 21, 2018

Description

Adds default slot rendering to v-progress-linear, so content can be added inside the progress bar.

Motivation and Context

Resolves #3768

How Has This Been Tested?

visually tested

Markup:

<v-progress-linear value="75" height="20" color="error">
    children
</v-progress-linear>

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any feature but make things better)

Checklist:

  • The PR title is no longer than 64 characters.
  • The PR is submitted to the correct branch (master for bug fixes, dev for new features and breaking changes).
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have created a PR in the documentation with the necessary changes.

@jacekkarczmarczyk jacekkarczmarczyk added the T: feature A new feature label Jun 8, 2018
@chewy94 chewy94 added the pending team review The issue is pending a full team review label Jun 12, 2018
@jacekkarczmarczyk jacekkarczmarczyk added the S: needs docs update The change requires an update to the documentation label Jun 13, 2018
Copy link
Member

@jacekkarczmarczyk jacekkarczmarczyk left a comment

Choose a reason for hiding this comment

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

Additionally I'm not sure whether then content should be vertically centered or maybe we should leave it to dev
Alternatively if the slot content is just a text it could be automatically centered, otherwise it's up to user to style it

@@ -109,6 +109,15 @@ export default {
this.genBar(h, 'long'),
this.genBar(h, 'short')
])
},
genContent (h) {
const children = [this.$slots.default]
Copy link
Member

Choose a reason for hiding this comment

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

      return this.$slots.default && h('div', {
        staticClass: 'v-progress-linear__content'
      }, this.$slots.default)

position: absolute
top: 0
left: 0
bottom: 0
Copy link
Member

Choose a reason for hiding this comment

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

left: 0 is not necessary

@KaelWD KaelWD removed the pending team review The issue is pending a full team review label Jun 30, 2018
@spezzino
Copy link
Contributor Author

spezzino commented Jul 6, 2018

Changes applied @jacekkarczmarczyk

@codecov
Copy link

codecov bot commented Jul 6, 2018

Codecov Report

Merging #4106 into dev will increase coverage by 0.7%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##              dev   #4106     +/-   ##
========================================
+ Coverage   88.89%   89.6%   +0.7%     
========================================
  Files         227     206     -21     
  Lines        5376    4876    -500     
  Branches     1360    1230    -130     
========================================
- Hits         4779    4369    -410     
+ Misses        479     408     -71     
+ Partials      118      99     -19
Impacted Files Coverage Δ
src/components/VProgressLinear/VProgressLinear.js 100% <100%> (ø)
src/mixins/dependent.js 54.16% <0%> (-45.84%) ⬇️
src/components/VMenu/mixins/menu-activator.js 44.44% <0%> (-3.71%) ⬇️
src/components/VForm/VForm.js 83.78% <0%> (-3.18%) ⬇️
src/components/VDataTable/VEditDialog.js 20% <0%> (-2.59%) ⬇️
src/mixins/menuable.js 93.13% <0%> (-2.01%) ⬇️
src/mixins/validatable.js 91.8% <0%> (-1.85%) ⬇️
src/mixins/selectable.js 93.87% <0%> (-1.58%) ⬇️
src/components/VTooltip/VTooltip.js 85.71% <0%> (-1.25%) ⬇️
src/components/VTextField/VTextField.js 93.22% <0%> (-0.85%) ⬇️
... and 188 more

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 214e561...f4456a6. Read the comment docs.

@johnleider
Copy link
Member

Please resolve conflicts.

@johnleider johnleider added the S: on hold The issue is on hold until further notice label Sep 2, 2018
@johnleider johnleider self-assigned this Sep 2, 2018
@johnleider johnleider added this to the v1.3.0 milestone Sep 2, 2018
@nekosaur
Copy link
Member

@spezzino please resolve conflicts and remove irrelevant commits

@johnleider
Copy link
Member

Closing due to inactivity.

@johnleider johnleider closed this Sep 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S: has merge conflicts The pending Pull Request has merge conflicts S: needs docs update The change requires an update to the documentation S: on hold The issue is on hold until further notice T: feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants