-
Notifications
You must be signed in to change notification settings - Fork 8
Adding new Product section with cards and tabs #17
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: znegrin <zurinegrin@gmail.com>
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.
Hi @znegrin Looking good! Just please be so kind as to add the carousel tabs and cards as UI library symbols also, inside the component you've just added. And let's wait for us to be able to see these components in the browser before we merge this.
Last, It seems you have some conflicts. Did you pull the repo before making the PR?
Thanks!
|
For sure I did the pull, obviously, and of course I added the blocks to the library. I add here a screenshot: Maybe it is something related to the conflict... Maybe it is happening because this is a file format that git cannot recognize and merge properly? |
|
Hi @znegrin ! The entire block yes, but not the tab navigation element, neither the card itself. It can be useful in the future :) You did a PR already, and I've merged it, with ease. There's must be something wrong with it. In alternative you can pull from the repo, and add the element again, it's already designed. Just please make sure you can handle this conflict. Thanks! |
Signed-off-by: znegrin <zurinegrin@gmail.com>
|
Hi @ricardobaeta, I just added a commit with the additions that you requested and also trying yo fix the conflicts. Please let me know if it is fine now. Thanks |
|
Hi @znegrin Looking good! Make sure you pull from origin. Thanks!
|
|
I always pull from origin. I have no idea that you can pull in any other way, so maybe the problem is another thing. |
|
You should update your fork - .../znegrin/design.git - pulling from origin - .../src-d/design - if I'm not mistaken. |
|
@znegrin keep in mind this PR was originally |
|
Hi @marnovo Sorry I really don't understand. I updated master from origin, and then from there I updated the product-page-update branch with the requested changes, which is the branch that needs to be compare right? |
|
@ricardobaeta @marnovo it seems that the problem is not a "real" conflict, but as I suggested, there is a problem between Git and binary files that always get a conflict like this one. I found some information about that: https://codeyarns.com/2016/04/11/how-to-resolve-git-conflict-with-binary-files/ I also found what seems to be a tool that can be helpful to use Git with .sketch files: It come from this: https://github.com/mathieudutour/git-sketch-plugin Maybe it is something we could try. For this one I suggest to just accept it after opening and reviewing the files. What do you think? |
|
@ricardobaeta asked me for some insights into what's happening here: $ git log
* 7639c1e 25-ene-2019 18:45 znegrin (HEAD -> PR17-product-page-update)
| - Fixing merge and adding individual elements
|
* 2997803 24-ene-2019 18:05 znegrin
| - Adding new Product section with cards and tabs
|
| * 17f2383 21-ene-2019 18:43 Ricardo Baeta (origin/master)
| |\ - Merge pull request #16 from znegrin/engine-modules
| / /
|/ /
| * 65f6294 21-ene-2019 19:31 Zuri Negrín
| | - Adding the new layout for the sub-section advantages to the pattern library
|/
|
* 48c9aa6 21-ene-2019 12:31 Marcelo Novaes (old-master)
| - Merge pull request #15 from src-d/fix-theme-link
|As @marnovo pointed it out (#17 (comment)), the branch being pullrequested This PR#17 modifies: $ git diff old-master..PR17-product-page-update
- ui-pattern-library/files/landing-pattern-library.sketch
- ui-pattern-library/files/typography.sketch
- ui-pattern-library/files/ui-elements.sketchbut it is not aware of changes already done in the previous PR #16 over the same files: $ git diff old-master..origin/master
- ui-pattern-library/files/landing-pattern-library.sketch
- ui-pattern-library/files/typography.sketch
- ui-pattern-library/files/ui-elements.sketchSince this PR is trying to modify files that were already modified in
What it is said at https://codeyarns.com/2016/04/11/how-to-resolve-git-conflict-with-binary-files/ (as @znegrin linked), is only considering the first two options, but it will imply that the work of one or the other side will be lost. In most cases, both work —from both sides— needs to be kept, so it is mandatory to manually apply the changes done in the other side before merging. Only when you know that the changes in your branch already contains the ones in the another, you can "ignore" the others. Was this explanation enough? Can I help you in any other way? |
|
addendum To avoid facing this problem once and again and again, people use to avoid modifying in parallel (in two different branches) the very same file (because the branch being merged first will win, and the second branch will face the problem). |
|
Thank you for the information @dpordomingo. :) I think the version in this PR is the most updated one right now, but it is only my guess, as I added to the files all the new and missing blocks, so as I said before, I think we can keep this one because it has all the new modules. What do you think @ricardobaeta @marnovo ? |




Here are the updated contents of the UI Pattern Library to include the new block of categories and cards to show source{d} Engine snippets on the Product page.
It is related to the document Landing Quick fixes & improvements and the issue #1340 of the backlog.
Signed-off-by: znegrin zurinegrin@gmail.com