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

Fixed #3859. Added help field in title tag for every textarea and input generated #3868

Closed
wants to merge 3 commits into from

Conversation

javitoron
Copy link
Contributor

Description

Added title='{{$vardef.help}}', title='{$fields.{{$city}}.help}', title='{$fields.{{$state}}.help}'...
You set help for address fields in Studio but then is not displayed when you go into the EditView.

Motivation and Context

Displaying help text in address auto generated fields

How To Test This

You set help for account billing address and billing city. then you go to new account and mouse over billing address and billing city.

Types of changes
[X] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[X] Breaking change (fix or feature that would cause existing functionality to change)

Final checklist
[X] My code follows the code style of this project found here.
[X] My change requires a change to the documentation.
[X] I have read the How to Contribute guidelines.

samus-aran and others added 3 commits June 30, 2017 16:40
Added help field in title tag for every textarea and input generated
…o-generated)-not-displaying-help

Update en_us.EditView.tpl
@Dillon-Brown Dillon-Brown added the Status:Assessed PRs that have been tested and confirmed to resolve an issue by a core team member label Jul 10, 2017
Copy link
Contributor

@daniel-samson daniel-samson left a comment

Choose a reason for hiding this comment

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

Review: Thank you for your contribution. Please review my requested changes?

@@ -1,6 +1,6 @@
## SuiteCRM 7.9.1
## SuiteCRM 7.9.2
Copy link
Contributor

Choose a reason for hiding this comment

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

Please only include the fix in your pull request. Please remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, newbie here... I only included the changed file... do you want me to include only the changed lines? not sure about how to do that...

and what do I have to remove?

$suitecrm_version = '7.9.2';
Copy link
Contributor

Choose a reason for hiding this comment

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

Please only include the fix in your pull request. Please remove.

$suitecrm_version = '7.9.2';
$suitecrm_timestamp = '2017-06-30 17:00';
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not change the time-stamp.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

??? :-(

@pgorod
Copy link
Contributor

pgorod commented Jul 15, 2017

@javitoron I know what you did wrong because it has happened to me before...

When you start your PR, you have to pay attention to where you are starting it from, because if it is the incorrect branch, or if that branch in your repo is not up to date with the branch on salesagility/SuiteCRM, then you get problems like this. You see, a PR always includes every difference between two branches, not just the changes you did deliberately to fix something.

So the normal workflow for you would be to first bring your hotfix branch up-to-date with salesagility/SuiteCRM:hotfix, then start a new branch in your repo (you must start it from within hotfix), make the changes, make the PR.

Now, that's the explanation; but to give you some practical steps to get you out of this mess, let me first ask: are you using only the online GitHub website for this, or do you have a local copy of the repo in Linux, that you can handle with git?

@javitoron
Copy link
Contributor Author

Yep, I have my own git in my server and I only use github to share the fixes I'm making there...

@pgorod
Copy link
Contributor

pgorod commented Jul 15, 2017

Good. Check out this blog post of mine, it shows how to bring the hotfix branch up to date.

You can either run the whole script as a fire-and-forget thing in a new directory, or just look inside the script for the commands to use in your current git repo (without needing to fetch everything down again).

But after you update hotfix you still have to move your commits from your master into a new branch (tip: never use master in these workflows). Here you have two options again:

  • either use all available git-fu in the universe doing this,

  • or do it the lazy way. Since your commit touches only one file, just go into that changed file and copy all its contents. Now checkout your own hotfix, start a new branch from there called patch-help-fields or something, edit the file pasting all the new contents, commit it, push it up to origin. Now edit this PR to get it's stuff from javitoron/SuiteCRM:patch-help-fields by changing it's head branch here on SalesAgility's GitHub.

Don't worry, git starts feeling easy after a while (like 14 months or something) :-)

@javitoron
Copy link
Contributor Author

soooo.... ok... could I just delete the repository and start a new one forked from salesagility:hotfix??

@daniel-samson
Copy link
Contributor

daniel-samson commented Jul 17, 2017

@jamesryanalexander You shouldn't have to delete your fork. Here is what to do.

So when you clone your fork down to a new local repository then add a new remote called upstream and point it the salesagility repository. Example

$ git clone https://github.com/javitoron/SuiteCRM.git SuiteCRM
$ git remote add upstream https://github.com/salesagility/SuiteCRM.git
$ git fetch --all

if you already have cloned down your fork, then you can simply skip the first command.

Then checkout the upstream (sales agility) hotfix

$ git checkout upstream/hotfix
$ git checkout -b hotfix

This will give you to the latest hotfix branch. You need to create a new branch from hotfix.

git checkout -b fix_3859

Then all you need to do is make the changes again or you can cherry pick the changes you have made.

In future, when you need to create a new fix, simply update your hotfix branch then create a new branch from hotfix:

$ git checkout hotfix
$ git fetch --all
$ git reset --hard upstream/hotfix
$ git checkout -b new_branch_name

when you are making new features you simply do that same command but change them from hotfix to develop.

@samus-aran samus-aran added the Status:Requires Updates Issues & PRs which requires input or update from the author label Jul 19, 2017
@javitoron
Copy link
Contributor Author

well... It doesn't matter, It's embarrasing but I haven't been able to follow your instructions, mainly because I don't know how to connect to github using a console.

I have my git server and I barely put to work my git repository with production and develompent instances.

And I just wanted to pay back with this project with the fixes I make... that's why I used github... and I only use it for that.

Sure I'm missing something and I really appreciate your help, but I can't lose more time trying to set up the repository... so I ended deleting the repository, where I only had my pull requests and forked from hotfix to pull some new fixes I have...

Thanks!

@javitoron
Copy link
Contributor Author

Hi @samus-aran! is there anything else I can/must do?

@javitoron
Copy link
Contributor Author

Is this going to be merged or should I make the request again from the correct branch? or try it at least...

@Dillon-Brown
Copy link
Contributor

Hi @javitoron, there are few code review comments that still need fixed. In this PR you have included README.md and suitecrm_version.php which will need to be removed before this can get merged. Please see the comments by pgorod for why this occurred. Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status:Assessed PRs that have been tested and confirmed to resolve an issue by a core team member Status:Requires Updates Issues & PRs which requires input or update from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants