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

Button Text Change Done #490

Closed
wants to merge 1 commit into from
Closed

Button Text Change Done #490

wants to merge 1 commit into from

Conversation

rajcrk
Copy link

@rajcrk rajcrk commented Nov 22, 2018

Fixes #[Add issue number here.]

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts
  • PR is descriptively titled
  • ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

@welcome
Copy link

welcome bot commented Nov 22, 2018

Thanks for opening this pull request! Dangerbot will test out your code and reply in a bit with some pointers and requests.
There may be some errors, but don't worry! We're here to help! 👍🎉😄

@harshkhandeparkar
Copy link
Member

harshkhandeparkar commented Nov 22, 2018

@rajcrk please remove

#[Add issue number here.]

and please swap it with the issue link.

Fixes https://link-to-the-issue

@harshkhandeparkar
Copy link
Member

Also it would be cool if you change the text besides the button which reads "press save to see changes".

@@ -130,7 +130,7 @@ function DefaultHtmlStepUi(_sequencer, options) {
$(step.ui.querySelectorAll(".target")).on('change',toggleSaveButton);

$(step.ui.querySelector("div.details")).append(
"<p><button class='btn btn-default btn-save' disabled = 'true' >Save</button><span> Press save to see changes</span></p>"
"<p><button class='btn btn-default btn-save' disabled = 'true' >Apply</button><span> Press save to see changes</span></p>"
Copy link
Member

Choose a reason for hiding this comment

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

You can change the text besides the button. That will look better.

@harshkhandeparkar
Copy link
Member

Great work by the way. You'll have to wait until @publiclab/reviewers review it.

Copy link
Member

@tech4GT tech4GT left a comment

Choose a reason for hiding this comment

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

This looks great!! Please change the span text as well and I think we will be done!! Thanks! 😄

@Rishabh570
Copy link

Hi @rajcrk, can you please change the span text as suggested? This is good to go after that...

@grvsachdeva
Copy link
Member

Hi @rajcrk, just checking if you need any help. Thanks!

@harshkhandeparkar
Copy link
Member

These changes have been applied already. Maybe in some other PR. Can this one be closed? @jywarren @gauravano

@grvsachdeva
Copy link
Member

Thanks @harshkhandeparkar for the status update. Closing this PR as master contains the required chages and some work is tracked in #517. Thanks @rajcrk for working on it!

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.

None yet

5 participants