BeforeIDieCode / BeforeIDieAchievements

🌟An open source project that helps developers make their first pull request and contribute to open source projects. πŸš€πŸŽ― Developers can contribute by sharing what they want to do before they die. πŸ’‘πŸ”—
https://before-i-die-achievements.vercel.app
MIT License
114 stars 178 forks source link

customized scrollbar for wen-kit browsers #59

Closed negar-75 closed 1 year ago

negar-75 commented 1 year ago

solved scrollbar issue

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
before-i-die-achievements βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback Sep 6, 2023 9:25pm
XanderRubio commented 1 year ago

@negar-75 I went ahead and connected this pull request to #48 so that once this gets merged, it will close out this issue officially. The scroll bar issue is good to go now βœ…, and the only issue I still see is the div's alignment with the contributors' bid images. It is off to the left. And I'm still not sure why. I want to hold off merging until we figure it out. Let me know your thoughts.

https://github.com/BeforeIDieCode/BeforeIDieAchievements/assets/120526253/5724db4a-7552-4b05-9413-91688ff01c7e

negar-75 commented 1 year ago

https://github.com/BeforeIDieCode/BeforeIDieAchievements/assets/113235504/246246ce-cb84-4b72-baa7-1ca89d3b488c

I can not understand where this problem comes from because I see nothing from the issueπŸ˜‚πŸ˜‚ I removed padding and margin, did you see that?

negar-75 commented 1 year ago

you are using safari?

XanderRubio commented 1 year ago

bandicam-2023-09-06-12-24-50-474_2tENSTDh.mp4 I can not understand where this problem comes from because I see nothing from the issueπŸ˜‚πŸ˜‚ I removed padding and margin, did you see that?

Hahaha, incredible! A mysteryπŸ˜… Thank you for taking the time to share your screencast and I did see that you removed the padding and margin. I have an idea that I'm going to look into and see if that will fix it on my side. I'll respond back shortly.

you are using safari?

I have been using Chrome

XanderRubio commented 1 year ago

I figured it out! After reviewing againπŸ˜… I've experienced numerous times when needing to find the fix, stepping away and returning to it with fresh eyes. After several days, we figured it out! I noticed now, viewing the issue#48 branch in my VSCode and running the local server, that we were missing the "className="flex justify-content-center" in the div in App.js.

Go and add this to your App.js div:

<div className="flex justify-content-center" style={{ marginTop: "50px", padding: '50px' }}>

App.js from your main branch on your forked version

Screenshot 2023-09-06 at 14 45 03

App.js from your issue#48 branch

Screenshot 2023-09-06 at 14 45 26

@negar-75 Go ahead and this code into the App.js of the issue#48 branch and recommit and then we can merge Bottle with Popping Cork

Screenshot 2023-09-06 at 14 46 13
negar-75 commented 1 year ago

ok, I will do that, but there is another question, when we are not using libraries like tailwindcss or bootstrap, how changing className can make a change in our style??

XanderRubio commented 1 year ago

when we are not using libraries like tailwindcss or bootstrap, how changing className can make a change in our style??

That's a great question! I haven't given it much thought before πŸ˜…. I am aware of changing the className attribute of an element can still affect its style. When conducting research the className attribute is used to apply CSS classes to the element, which are defined in our external stylesheets or CSS modules containing our specific styling rules for elements with certain class names.

When you modify the className of an element, you're essentially changing the CSS class applied to it. This may result in different styling rules being applied, which can alter the element's appearance. This is what was happening when we were reviewing and seeing the change in where contributor's images were being aligned. The styles aren't defined inline within the component; instead, they're defined in separate CSS files or CSS modules, which are imported and applied to the components using the className attribute.

Currently, if our project does not use libraries such as Tailwind CSS or Bootstrap (we absolutely can integrate), altering the className of an element in a React project can impact its style. This is because the className implements predetermined CSS classes from external stylesheets or CSS modules. I hope this explanation has been useful! πŸ˜‡ Let me know if this helped, and please do not hesitate to let me know of any corrections that may be necessary. And thank you for the question and for thinking deeper about React.

negar-75 commented 1 year ago

I totally understand , these are global className from index.css file

negar-75 commented 1 year ago

I am so looking forward to see final result 😍😍😍😍😍😍

negar-75 commented 1 year ago

so what happened?

negar-75 commented 1 year ago

Revert "Issue https://github.com/BeforeIDieCode/BeforeIDieAchievements/issues/48" all my changes that I applied has been reverted , I can not see any piece of my code . after git pull I noticed all my changes has gone. I do not know what is happening

XanderRubio commented 1 year ago

so what happened?

@negar-75 πŸ˜…πŸ˜… I'm grateful for your time working together during this process as I've learned more as I thought this was going to be a very simple merge request, and glad it hasn't been to learn more and understand betterBeaming Face with Smiling Eyes.

I went ahead and solved conflicts, and then this merged the main into the issue-#48 branch, and this caused an issue as it didn't include your great contributions. Then I brought the issue-#48 branch back to get back to where we were before (with reverting back to your original commit of added className, and deployment now shows your changes, and they look great.

I need to review the branch again to resolve issues. I see you went ahead and resolved the merge conflict, and for some reason, it merges the entire main into issue#48, and this is what happened before. All good we can get back to the original state with your edits. I find it weird why resolving the conflict is doing this and there isn't another option, as it is a simple change in the App.js and module.css that is causing the merge conflict. The main takeaway is your code is saved and in the commit history even though the deployment preview shows otherwise.

XanderRubio commented 1 year ago

Revert "Issue #48" all my changes that I applied has been reverted , I can not see any piece of my code . after git pull I noticed all my changes has gone. I do not know what is happening

Let me organize the steps for you to implement to revert back your commit as this is a great learning opportunity for future times you need to restore your code and this will get the deployment preview back showing your changes😎

negar-75 commented 1 year ago

so should I apply all changes that I did before again? or they still exists?

XanderRubio commented 1 year ago

so should I apply all changes that I did before again? or they still exists?

Your changes still exist and we will get the issue-#48 branch back to this commit⬇️

Screenshot 2023-09-06 at 20 10 23
negar-75 commented 1 year ago

It is no problem, so we will get back to"added className to div " commit?

XanderRubio commented 1 year ago

It is no problem, so we will get back to"added className to div " commit?

Yes we will get it back to where it was before this is a good learning lesson😎

XanderRubio commented 1 year ago

To revert back to the working commit of added className to div:

  1. from your terminal in VSCode run

    git log
  2. Then you see this

    Screenshot 2023-09-06 at 20 14 38
  3. Copy the hash number (just the number not the word commit)

  4. Then your going to run the command and replace the <commit-hash> with the hash number. This command will move the HEAD and the current branch pointer to the specified commit, discarding all commits that came after the specified commit.

    
    git reset --hard <commit-hash>
6. Lasty running the following command to push the changes to the remote repository, you'll need to force-push the updated branch

git push -f



Let me know if I can help with clarify or anything else πŸ˜‰
negar-75 commented 1 year ago

in issue-#48 branch should I do that?

XanderRubio commented 1 year ago

in issue-#48 branch should I do that?

yes in branch issue-#48

negar-75 commented 1 year ago

I am done

negar-75 commented 1 year ago

Screenshot 2023-09-06 193122

so now how can I resolve this conflict?

XanderRubio commented 1 year ago

Screenshot 2023-09-06 193122

so now how can I resolve this conflict?

Now, when you recommit again, this will redeploy the CI (Continuous Integration) to the preview link and we will see it working. You can recommit with a message Redoplying as an example and will have to make some small change to your branch for you to make a new commit.

I'm unsure how to resolve the conflict in GitHub without merging the main into the branch issue-#48 (from the prompt that is given). One solution is to see if we can perform this merge currently from the command line or another solution that come to mind would be opening another pull request with issue-#48, fixing the conflict changes ahead of time as a possible solution. What do you think?

XanderRubio commented 1 year ago

I know we can solve this merge conflict from the terminal following these command that are given to us but I do believed we need to change it slightly or else it causes the main to merge in issue-#48 when it should be the other way around. git checkout -b 'negar-75-issue-#48' main git pull https://github.com/negar-75/BeforeIDieAchievements.git 'issue-#48' Step 2: Merge the changes and update on GitHub.

git checkout main git merge --no-ff 'negar-75-issue-#48' git push origin main

XanderRubio commented 1 year ago

@negar-75 I will work on getting this merged by closing this pull request and opening another pull request. Thanks for your patience, and I will have an update shortly.

XanderRubio commented 1 year ago

@negar-75 Based on my testing of various solutions, I have decided to merge your issue#48 branch into the feature-48 branch on the main repository. This approach will help us avoid any merge conflicts. Upon reviewing the situation, I have come to the conclusion that the potential issue could be due to the upstream branch not being updated with the main repository before initiating a pull request on the forked repo. To prevent this issue from arising in the future, I will be adding documentation on the process of submitting pull requests for new features with other branches. Please stay tuned as I work towards getting your code up and running on the live link.

negar-75 commented 1 year ago

I totally got confused with reverting and these stuff, I think this is better to create another branch and apply all changes from scratch on that branch and then create another pull request. what do you think?

XanderRubio commented 1 year ago

I totally got confused with reverting and these stuff, I think this is better to create another branch and apply all changes from scratch on that branch and then create another pull request. what do you think?

Agreed! Also better to create a new branch as a feature branch and then create another pull request. Additionally, making sure the main is fetch and fully up to date and then creating a new branch to work on can help with limiting potential merge conflict.