-
Notifications
You must be signed in to change notification settings - Fork 55
feat(view): add PackageSummary #38
base: latest
Are you sure you want to change the base?
Conversation
lib/components/view.jsx
Outdated
|
|
||
| class PackageSummary extends Component { | ||
| render () { | ||
| let { packument, spec, json } = this.props |
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.
What's the packument thing? Looking at getData, it looks like it's a different format than what comes back for search results—here's an example:
{
package: {
name: 'goog',
scope: 'unscoped',
version: '0.2.1',
description: 'Server-side Google Closure with Node.js',
links: {
npm: 'https://www.npmjs.com/package/goog',
homepage: 'https://github.com/hsch/node-goog#readme',
repository: 'https://github.com/hsch/node-goog',
bugs: 'https://github.com/hsch/node-goog/issues'
},
author: {
name: 'Hendrik Schnepel', url: 'hendrik.schnepel@gmail.com'
},
publisher: {
username: 'hsch',
email: 'hendrik.schnepel@gmail.com'
},
maintainers: [ { username: 'hsch', email: 'hendrik.schnepel@gmail.com' } ]
},
flags: { unstable: true },
score: {
final: 0.0333453077955526,
detail: {
quality: 0.10500992679095593,
popularity: 0.0052637993093309285,
maintenance: 0
}
},
searchScore: 100000.03
}Unless this is being reused elsewhere, it would be nice if this component accommodated that format, unless I'm missing something about packument...
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.
Completely forgot about this, sorry. The packument is what the npm APIs return when querying for a package, but to use that in the search result would require additional API calls which isn't ideal for a responsive interface. I'll add a PackageSearchResult component.
|
This looks great! Happy to merge this pending @chrisforrette's comments and the rest of your TODO 👍 |
8714577 to
cc587e8
Compare
|
I added a |
chrisforrette
left 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.
Looks great!
|
@chrisforrette right, I didn't notice this wasn't working |
Add a PackageSummary for a consistent 3-line information-dense summary of a package.
cc587e8 to
7bc6f79
Compare
|
@zkat do I need to do anything else before merge? |
|
@zkat Hey Kat! Can we merge this thinger? |
Add a
PackageSummaryfor a consistent 3-line information-dense summary of a package. WithBox:Without (not possible currently, I didn't include it due to the way it looks with other content around it):
Some TODOs:
I think the other thing was "wrap lines if too long", but that's completely contradictory, so scratch thatSee #11 (comment) (@chrisforrette)