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

Material ItemMeta text-only #1337

Open
fopina opened this issue Sep 29, 2019 · 11 comments
Open

Material ItemMeta text-only #1337

fopina opened this issue Sep 29, 2019 · 11 comments

Comments

@fopina
Copy link
Contributor

fopina commented Sep 29, 2019

List.ItemMeta only supports icons though ItemMeta in material design allows for text only.

I can make a PR but not sure how to handle non-breaking changes.
Different component (ItemMetaText?)
or a bool prop with no-icon which defaults to false (so that default behavior is the existing one)?

@prateekbh
Copy link
Owner

@fopina
Copy link
Contributor Author

fopina commented Oct 8, 2019

thanks for the quick response @prateekbh
where is the branch/tag 1.6.0 so I can start by changing that one (as it's the one I use in my app)?
Then I can do it for v2-alpha as well

It seems 1.5.8 and 1.6.0 are available in npmjs but not here in github...

@prateekbh
Copy link
Owner

The PR should be to branch 2.0

@fopina
Copy link
Contributor Author

fopina commented Oct 8, 2019

I’ll make the PR on branch 2, but I need to make the changes to 1.X as well, as that’s what I’m using. Though I don’t see a 1.6.0 tag...

@cromefire
Copy link
Collaborator

cromefire commented Oct 8, 2019

1.X is in master

fopina added a commit to fopina/preact-material-components that referenced this issue Oct 8, 2019
fopina added a commit to fopina/preact-material-components that referenced this issue Oct 8, 2019
fopina added a commit to fopina/preact-material-components that referenced this issue Oct 8, 2019
fopina added a commit to fopina/preact-material-components that referenced this issue Oct 8, 2019
@fopina
Copy link
Contributor Author

fopina commented Oct 8, 2019

So #1338 and #1339 done to cover 1.x and 2.0.
I've opted for a new component ItemMetaText over a flag in the existing one as that'd require more changes (break existing inheritance) and I don't think a property text with default false or icon with default true (to keep retro-compatibility) would make much sense... but let me know your thoughts and I can update

@fopina
Copy link
Contributor Author

fopina commented Oct 8, 2019

And, sort of off topic, I was trying to build the tarball for my branch such as the one published in npmjs, but I can't find the command (new to npm, preact, etc) nor it seems documented. And as it is not published with travis, can't find it there either...
npm run build puts quite a few items in the current directory, not sure how to pack only what matters (to make it only 2.6M in size as the published one)

@cromefire
Copy link
Collaborator

Run yarn build and then yarn pack (we use yarn)

@fopina
Copy link
Contributor Author

fopina commented Oct 8, 2019

Thanks @cromefire got my tarball now (quick responses!)

1.X is in master

Regarding this, https://github.com/prateekbh/preact-material-components/blob/master/package.json#L3 so no idea where to get the version 1.6.0 that is available in npmjs...

@cromefire
Copy link
Collaborator

@prateekbh You published that right?

@prateekbh
Copy link
Owner

That is master, just that I didn't bump the version here

prateekbh pushed a commit that referenced this issue Oct 16, 2019
* add support for non-icon List ItemMeta #1337

* List.ItemMetaText in docs

* docs List test updated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants