-
-
Notifications
You must be signed in to change notification settings - Fork 228
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
Compare the size of multiple packages #202
base: bundlephobia
Are you sure you want to change the base?
Compare the size of multiple packages #202
Conversation
Related: #78 |
This looks really cool! |
@styfle Awesome 👍 |
} | ||
} | ||
|
||
handleInputChange = ({ target }) => { | ||
const { value } = target; | ||
this.setState({ value: target.value }); |
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.
value is already destructured from the target. can we do this.setState({ value })
?
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.
Yep my bad. I didn't notice it.
|
||
if (trimmedValue.length > 1) { | ||
this.getSuggestions(name) | ||
if (value === item.package.name) return |
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.
; at the end.
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.
👍
@styfle Hey can you check the implementation, ui and let you know. If all good you can merge it else we can do some improvements or any changes :) |
@gokulkrishh I don't own this repo 😄 Perhaps you are confusing Bundle Phobia and Package Phobia? 😉 |
@styfle Yep my bad. I meant to ask @pastelsky |
@pastelsky Did you checked my PR ? |
Hey @gokulkrishh , thanks for sending out this PR. Also the use of A few questions for you –
Also, let's discuss solutions before you spend more effort working on code. |
@pastelsky. I'm glad you liked it. Yes i followed And regarding your questions Question 1: Good question 👍, as of now user have to manually enter the version number in search box once he/she finds the result in autocomplete and can compare like the same. I think no new changes required to make this happen. Screenshot: Question 2: Yes like you said, ux is not that great because the input box scale's down and its hard to read pkg names in input box if its more than
Initially I was thinking we should have tags in input box or below input box for comparing packages. Screenshot: But not so sure that would be great. Let me know your thoughts too. I will try to come up with other ideas and see what will work best for bundlephobia. |
|
Also, when does the |
Plus i have to see how it looks in mobile view and fix that too. Last Q: Yes, compare button comes when user add's |
I have added limit in above commits and some fixes for mobile screen. But issue |
@pastelsky Hey, i have added some changes to support highlight Can you take a look into the pr and let me know if this looks good. Test for util is pending which i will write once everything is okay. |
@pastelsky Did you got time to check the latest changes ?? |
I've been pushing a slightly large refactor of Bundlephobia — adding a few
new APIs and restructuring some code over the last few days, hence wanted
to hold this off incase that required a revert.
I'll do a second pass and probably rajde a couple of small UI improvements
to this (if you don't mind) and merge it then.
Again, thanks for your patience.
…On Wed, Sep 4, 2019, 2:34 PM Gokulakrishnan Kalaikovan < ***@***.***> wrote:
@pastelsky <https://github.com/pastelsky> Did you got time to check the
latest changes
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#202?email_source=notifications&email_token=ACEIEHZRRJSZPB4WGLVKTF3QH52YNA5CNFSM4IKMURNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD524NJQ#issuecomment-527812262>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACEIEH7UZBJDHAYJUBZRBRDQH52YNANCNFSM4IKMURNA>
.
|
@pastelsky Sure. I dont mind at all. This feature is much need one and it can wait till your refactoring and ya change the UI if it need improvements ;) |
I just want to chip in, I also can't wait for this feature! It will improve the lives of many developers; leading to better websites; leading to a better internet! |
8dd12c5
to
5efa4b7
Compare
7572841
to
1d11eaf
Compare
What happened to this pull request? |
yes!!! it was approved!!! |
Done:
This is initial idea only from @egoist from this issue styfle/packagephobia#133 . I believe we can improve it. @styfle let me know your thought :)
Idea:
Attaching a screenshot and GIF for reference on how it works.
Screenshot:
GIF: