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

Compare the size of multiple packages #202

Open
wants to merge 6 commits into
base: bundlephobia
Choose a base branch
from

Conversation

gokulkrishh
Copy link

@gokulkrishh gokulkrishh commented Aug 8, 2019

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:

  1. An option to search multiple package in search with comma.
  2. New package search in search bar will now show compare button to compare packages.
  3. After clicking search, if the values in search bar are comma separated the page will go to compare packages page.

Attaching a screenshot and GIF for reference on how it works.

Screenshot:

Screenshot 2019-08-08 at 9 42 21 PM

GIF:

Bundle Phobia

@gokulkrishh
Copy link
Author

Related: #78

@styfle
Copy link
Contributor

styfle commented Aug 8, 2019

This looks really cool!

@gokulkrishh
Copy link
Author

@styfle Awesome 👍

}
}

handleInputChange = ({ target }) => {
const { value } = target;
this.setState({ value: target.value });

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 })?

Copy link
Author

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

Choose a reason for hiding this comment

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

; at the end.

Copy link
Author

Choose a reason for hiding this comment

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

👍

@gokulkrishh
Copy link
Author

gokulkrishh commented Aug 9, 2019

@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 :)

@styfle
Copy link
Contributor

styfle commented Aug 9, 2019

@gokulkrishh I don't own this repo 😄

Perhaps you are confusing Bundle Phobia and Package Phobia? 😉

https://github.com/styfle/packagephobia

@gokulkrishh
Copy link
Author

@styfle Yep my bad. I meant to ask @pastelsky

@gokulkrishh
Copy link
Author

@pastelsky Did you checked my PR ?

@pastelsky
Copy link
Owner

pastelsky commented Aug 19, 2019

Hey @gokulkrishh , thanks for sending out this PR.
First thoughts on this feature – I love it. I think this plays nicely with the package.json scan UI you've used to display the results of multi-package searches.

Also the use of , to delimit packages is exactly how we form the URL, which is good.

A few questions for you –

  1. Can I still specify specific versions of packages? Eg. Can I enter [email protected], [email protected] in the search box? If yes, how do we handle highlighting of versions?

image

  1. I'm not sure the current search box is going to scale well for entering multiple packages – package names can get very long, and they get even longer with version numbers. Mobile is a different game altogether.
    We might need a better UX for cumulating the package names – either as chips inside the search box, or a different way.

Also, let's discuss solutions before you spend more effort working on code.

@gokulkrishh
Copy link
Author

gokulkrishh commented Aug 19, 2019

@pastelsky. I'm glad you liked it.

Yes i followed , separated delimiter based on how it already works.

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:

Screenshot 2019-08-19 at 5 46 55 PM

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 5 pkg's
Screenshot:

Screenshot 2019-08-19 at 5 51 59 PM

We might need a better UX for cumulating the package names – either as chips inside the search box, or a different way. - yes exactly we need to find a better ux to represent the compared pkgs in search page.

Initially I was thinking we should have tags in input box or below input box for comparing packages.

Example from dribble:

Screenshot:

Screenshot 2019-08-19 at 5 56 20 PM

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.

@pastelsky
Copy link
Owner

@gokulkrishh

  1. Yes, the version numbers can be entered manually, but notice that the syntax highlighting is off when multiple packages are entered.

  2. Let's limit the maximum number of packages here to 3 here then.

@pastelsky
Copy link
Owner

Also, when does the compare label show up in the auto-suggestions? Are they always there, or they appear after you enter the first ,? The latter would be preferable.

@gokulkrishh
Copy link
Author

gokulkrishh commented Aug 20, 2019

@pastelsky

  1. yes it is off when there are more. I will fix it.
    2.Sure we can do that. If people ask for more then probably we can add more to limit.

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 , next to package.

@gokulkrishh
Copy link
Author

I have added limit in above commits and some fixes for mobile screen.

But issue version numbers syntax highlighting is yet to be fixed.

@gokulkrishh
Copy link
Author

@pastelsky Hey, i have added some changes to support highlight @ for multiple packages.

Screenshot:
Screenshot 2019-08-26 at 6 43 17 PM

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.

@gokulkrishh
Copy link
Author

gokulkrishh commented Sep 4, 2019

@pastelsky Did you got time to check the latest changes ??

@pastelsky
Copy link
Owner

pastelsky commented Sep 6, 2019 via email

@gokulkrishh
Copy link
Author

gokulkrishh commented Sep 6, 2019

@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 ;)

@mesqueeb
Copy link

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!

@pastelsky pastelsky force-pushed the bundlephobia branch 2 times, most recently from 7572841 to 1d11eaf Compare June 6, 2021 11:43
@okikio
Copy link

okikio commented Jun 20, 2021

What happened to this pull request?

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

Successfully merging this pull request may close these issues.

None yet

6 participants