Skip to content
This repository has been archived by the owner on Mar 2, 2022. It is now read-only.

Frontend shubhamkakkar #64

Closed

Conversation

shubhamkakkar
Copy link

Upgraded vue and related dependencies, used vuetify as css based framework.

Example of work done
Upgraded to latest versions (issue #61 )
Improved UI
Animated the logo and placed it on Landing Page, replacing the phone's image with it
Wisely utilzed white spaces
Same route's nav item not present ie: Removed Navbar item when on same route (issue #62 )
Used pushButton on each webpage (v-app) to prevent user's work to scroll, click would navigate to up / bottom component, which are strategically placed so that the user's mouse will finally fall on to GUIDE button (would navigate to /guide )

left: Responsive Navbar, Components
Help Needed: In Components Section

working link in readme

@DennisSchwartz
Copy link

Hi @shubhamkakkar!

Thanks for working on this. I can see you put in a lot of work. But I have a number of questions about this PR.

image
image

Is this what it's supposed to look like? Or is something not working for me?
You've basically rewritten the whole frontend but there are a number of issues that I can see right away. All the colors are different, the layout is entirely different and by far the most important part - the component registry - is missing.

I can't see how this adds value to our users. Maybe it would be great to have a call with @yochannah, myself and possibly @sarthak-sehgal and @Megh-Thakkar sometime next week to discuss the goals and needs of the BioJS community. This could give you a better insight into what we need.

Again, I really appreciate that you chose to spend time on our project and I'd love to incorporate some of your ideas. I can see that you have a great directory structure and a lot of enthusiasm in there.
But honestly, a PR with non-trivial changes to 158 files on a project you've never worked on before it not a good introduction. And - again, not trying to take away from the work you put in - a spinning logo is no additional use to our users as opposed to an improved search view or working visualisations for example.

@yochannah What do you think? Could we schedule a call some time. I don't see how we can merge this at the moment.

@yochannah
Copy link

@shubhamkakkar Hey Shubham! Thanks for mkaing this PR - we can see you've put a lot of work into it.

Echoing Dennis a little here: When contributing to open source projects it's usually best to try to make your contributions reasonably small first of all, to make sure you're familiar with the team's workflow. I don't think we expected quite such a big PR, and the site has actually only just been redesigned, so going for a whole new theme is a bit more than what we were looking for.

Examples of good small started issues might be upgrading the dependencies and making a PR for that, or perhaps working on this issue: #62

@shubhamkakkar
Copy link
Author

Hey, thanks for the review I will definately keep all your reviews in mind while working next time.

Yes @DennisSchwartz it is supposed to look that way, I was trying to improve the front end, yes the component's section is missing I am working over it.
As per "colors are different" : I do develop front end using different colors and all, was appreciated a lot in my work circle, would definately need to learn more and listen from you about your advice on it.
For spinnig logo - its just a substitute for the phone, yes nothing is in for the user, but better than still phone image(I thought)
Thanks for suggestions and realted advices

and @yochannah I will keep in mind your words as well

Thanks to both of you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants