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

1.9 unset fix #1561

Closed
wants to merge 4 commits into from
Closed

1.9 unset fix #1561

wants to merge 4 commits into from

Conversation

hdwebpros
Copy link
Contributor

Fixed unset bug on 1.9 version. Not sure how to do this cleaner. The two commits I did that are noteworthy are:
135387e
264967a

I have 4 clients that have websites on Grav that use the same images on more than one page. That is why they need unset. If they delete the image, it gets removed from other pages too. Unset just removes the image but allows it to be on other pages.

The bug right now is that if you have a blueprint with more than one area for images, unset effects everything and sets everything to one image. These commits fix that. Tested locally.

@rhukster
Copy link
Member

Why do you have 74 files changed Ryan? Did you start from admin develop branch? Should only have the files you actually modified.

@hdwebpros
Copy link
Contributor Author

Here was my process. Please let me know how I should do it differently.

  • Downloaded Grav and all other plugins (form, login, etc) and set up locally
  • Forked existing admin plugin repo (I'm only allowed to fork the main branch) and placed this forked repo in my local install
  • Cloned 1.9 branch of admin plugin
  • Move 1.9 branch files to my fork and push that to my main branch on my fork
  • Create a new branch on my fork for my fixes
  • Apply my fixes and push to my newly created branch
  • Create a PR through GitHub.

When I follow this procedure, it obviously doesn't work well. It applies my original pushes described in the step Move 1.9 branch files to my fork and push that to my main branch on my fork

How should I be doing this?

@w00fz
Copy link
Member

w00fz commented Dec 17, 2018

It’s a little bit hard to follow what’s going on and what changed

@hdwebpros
Copy link
Contributor Author

135387e

And

264967a

Do you need me to describe what was done in that commit that made it work?

@hdwebpros
Copy link
Contributor Author

hdwebpros commented Jan 8, 2019

Just following up on this.

But, I've explained every line in my main commit. I just have no way to do the PR against the 1.9 branch. I researched and I'd have to fetch upstream and do some other git related stuff that I flat out aren't that comfortable with or good at.

I appreciate any assistance here. This fix would help so many clients

@w00fz w00fz closed this Apr 17, 2019
@mahagr
Copy link
Member

mahagr commented Apr 25, 2019

@hdwebpros Everything is now merged to develop, so please try again. Github has very good documentation, so please read it, too. :)

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.

4 participants