-
Notifications
You must be signed in to change notification settings - Fork 9
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
P#61223 19600 reload edit pages after saving and show copyable shortcode there #160
P#61223 19600 reload edit pages after saving and show copyable shortcode there #160
Conversation
@jayay could you please review this PR? |
@jayay please check PR help me, thanks |
@jayay could you please review this PR? |
js/onoffice-copycode.js
Outdated
jQuery(document).ready(function($){ | ||
$('#button-copy').click(function() { | ||
var copyCode = document.getElementsByTagName("code").item(0); | ||
// console.log(copyCode); |
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.
please remove the debug code
|
||
$code = '[oo_estate view=="'.$listName.'"]'; |
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.
one =
too much
{ | ||
$id = HtmlIdGenerator::generateByString($this->_id); | ||
echo '<input type="button" class="button" id="button-copy" name="' | ||
.esc_attr($id).'" value="'.esc_html__('Copy !', 'onoffice-for-wp-websites').'" ' |
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.
Copy!
would be better
819190d
to
9b7cd74
Compare
@jayay i fixed, can you check PR help me ? |
6312cd3
to
da59f69
Compare
|
||
$codes = '[oo_estate view=="'.$listName.'"]'; |
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.
one =
too much here
|
||
public function testCreateInputModelButton() | ||
{ | ||
$pInstance = $this->getMockBuilder(FormModelBuilderDBEstateListSettings::class) |
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.
Testing a class using a mock of itself doesn't really make sense
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.
i fixed it, please check PR help me !
Steps to install the approved version:
|
This is the test video: Reload_edit_pages_after_saving_and_show_copyable_shortcode_there_2.mp4 |
…how-copyable-shortcode-there
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.
There's too many changes for me to review them, but I will test it.
Steps to install the approved version:
|
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.
I didn't explicitely ask before, but it would be great to also have a unified way of showing shortcodes.
-
The detail view does not use the new shortcode display:
It should look like this:
-
Similarly, when the detail view shortcode is not used
it should look like this:
-
Unit list settings do not show a shortcode.
They should have the same shortcode as the estate list settings (also the same behavior when creating a new unit list).
Do we apply the new display shortcode for other pages (estate list, address list, form, unit lits)? |
I'm not sure I understand. What do you mean? 😅 |
Steps to install the approved version:
|
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.
It all works great, thanks! Another PR finally finished. 🎉
No description provided.