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

feat(VGrid): pass down data-* attributes #6208

Merged
merged 4 commits into from Jan 29, 2019

Conversation

posva
Copy link
Contributor

@posva posva commented Jan 19, 2019

Description

Allow usage of data attributes like data-test="container" which are useful for testing and can be
removed via vue-loader plugins

Motivation and Context

fixes #3404

This allows usage of <v-layout data-test="foo"></v-layout> to then find them in tests with wrapper.find('[data-test=foo]') without needing to add extra classes that aren't used by any css. These attributes can be removed automatically so they do not exist in production code with a vue-loader plugin. There is a vue-cli plugin https://github.com/LinusBorg/vue-cli-plugin-test-attrs

How Has This Been Tested?

unit

Markup:

// Paste your FULL Playground.vue here

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 and documentation updates, dev for new features and breaking changes).
  • My code follows the code style of this project.
  • I've added relevant changes to the documentation (applies to new features and breaking changes in core library)

@vercel
Copy link

vercel bot commented Jan 19, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@posva
Copy link
Contributor Author

posva commented Jan 19, 2019

I haven't added any documentation changes. I will check what exists
edit: if there is a place where you talk about attributes being automatically converted to classes, then I could add a note there but I couldn't find the place in the docs

@codecov
Copy link

codecov bot commented Jan 19, 2019

Codecov Report

Merging #6208 into dev will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #6208      +/-   ##
==========================================
- Coverage   82.56%   82.55%   -0.01%     
==========================================
  Files         293      293              
  Lines        7019     7023       +4     
  Branches     1726     1727       +1     
==========================================
+ Hits         5795     5798       +3     
- Misses       1089     1090       +1     
  Partials      135      135
Impacted Files Coverage Δ
packages/vuetify/src/components/VGrid/grid.js 100% <100%> (ø) ⬆️
...fy/src/components/transitions/expand-transition.js 72% <0%> (-4%) ⬇️

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 82b68d1...70ee4ba. Read the comment docs.

@jacekkarczmarczyk
Copy link
Member

I don't think there is a place when we mention it, the best would https://vuetifyjs.com/en/framework/grid

dsseng
dsseng previously approved these changes Jan 20, 2019
@dsseng
Copy link
Contributor

dsseng commented Jan 20, 2019

@posva Thanks!

@MajesticPotatoe MajesticPotatoe dismissed dsseng’s stale review January 20, 2019 05:57

awaiting docs update

@dsseng dsseng added T: enhancement Functionality that enhances existing features T: feature A new feature S: needs docs update The change requires an update to the documentation labels Jan 20, 2019
@posva
Copy link
Contributor Author

posva commented Jan 21, 2019 via email

@MajesticPotatoe
Copy link
Member

@posva

Should I add some documentation tip in the page you mentioned or do you prefer to merge it and do it your way?

Would say add a documentation tip on the grid page jacek mentions, and should be good.

@posva
Copy link
Contributor Author

posva commented Jan 23, 2019

Ok! Will do it today probably

Allow usage of data attributes like data-test="container" which are useful for testing and can be
removed via vue-loader plugins
@posva posva force-pushed the feat/allow-data-attrs-vgrid branch from ffd8b99 to a810d73 Compare January 23, 2019 20:53
@posva
Copy link
Contributor Author

posva commented Jan 23, 2019

Added the docs, that was much harder than expected 😓 It isn't easy to understand the json data structure that generates the documentation. I also had this error all along: Can't resolve '@vuetify/api-generator'. I was probably missing a step somewhere

@MajesticPotatoe
Copy link
Member

@posva yarn && yarn build && yarn dev docs should would (build step will build the api-generator which is needed for the docs)

still working on improving structure. if you need assistance on navigating them, reach out to me on discord.

ill take a look at this when i get home later 👍 great work.

@johnleider johnleider removed the S: needs docs update The change requires an update to the documentation label Jan 29, 2019
@johnleider johnleider merged commit 198a3d4 into vuetifyjs:dev Jan 29, 2019
@posva posva deleted the feat/allow-data-attrs-vgrid branch January 29, 2019 15:49
@lock
Copy link

lock bot commented Apr 14, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. Please direct any non-bug questions to our Discord

@lock lock bot locked as resolved and limited conversation to collaborators Apr 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T: enhancement Functionality that enhances existing features T: feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants