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

42302 Can't remove/delete secrets and keys #680

Merged
merged 15 commits into from
Apr 15, 2024

Conversation

dai-eastgate
Copy link
Contributor

related to #677
changed log:
Add delete button and show keys

Copy link

github-actions bot commented Nov 20, 2023

Pull Request Test Coverage Report for Build 8683894277

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 28 of 33 (84.85%) changed or added relevant lines in 3 files are covered.
  • 204 unchanged lines in 13 files lost coverage.
  • Overall coverage increased (+0.01%) to 79.521%

Changes Missing Coverage Covered Lines Changed/Added Lines %
plugin/Renderer/InputFieldGoogleRecaptchaAccountRenderer.php 19 20 95.0%
plugin/Renderer/InputModelRenderer.php 0 4 0.0%
Files with Coverage Reduction New Missed Lines %
plugin/DataView/DataListView.php 1 98.31%
plugin/Installer/DatabaseChanges.php 1 99.52%
plugin/FormPostContact.php 5 91.3%
plugin/Renderer/InputFieldComplexSortableDetailListContentDefault.php 7 0.0%
plugin/Field/Collection/FieldsCollectionBuilderShort.php 8 56.96%
plugin/Form/FormAddressCreator.php 9 94.3%
plugin/Filter/DefaultFilterBuilderListView.php 10 81.67%
plugin/FormPostInterest.php 13 84.34%
plugin/EstateList.php 15 90.72%
plugin/FormPostOwner.php 17 85.47%
Totals Coverage Status
Change from base Build 8261543519: 0.01%
Covered Lines: 8131
Relevant Lines: 10225

💛 - Coveralls

@fredericalpers fredericalpers linked an issue Nov 24, 2023 that may be closed by this pull request
3 tasks
@fredericalpers fredericalpers added this to the v4.19 milestone Nov 24, 2023
@fredericalpers fredericalpers added the QA Issue or Pull request that is in review label Nov 24, 2023
@fredericalpers fredericalpers added UI/UX Issue, Pull Request or Discussion related to UI/UX component: dashboard Issue, Pull Request or Discussion related to dashboard / settings page labels Feb 20, 2024
Copy link
Contributor

@andernath andernath left a comment

Choose a reason for hiding this comment

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

@dai-eastgate I added some comments, please take a look :)

@@ -15,4 +15,19 @@ jQuery(document).ready(function ($) {

jQuery.post(warning_active_plugin_vars.ajaxurl, data);
});

$(document).on('click', '.delete-google-recaptcha-keys-button', function (event) {
if (confirm('Would you like to permanently delete the site key and the secret key?')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use a textkey which can be translated

@@ -0,0 +1,22 @@
jQuery(document).ready(function ($) {
function showOrHiddenGoogleRecaptchaKey(inputSelector, toggleSelector) {
Copy link
Contributor

Choose a reason for hiding this comment

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

showOrHideGoogleRecaptchaKey

@@ -0,0 +1,39 @@
<?php
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this renderer is necessary.
What do you think to put this button inside of InputFieldPassworkRenderer and maybe rename it? Or do you think this is bad practice and every single element has to be a seperated class?

css/admin.css Outdated
@@ -460,8 +460,48 @@ body #post-head-content p.wp-clearfix:nth-child(1) {
font-size: 14px;
white-space: nowrap;
}

.oo-google-recaptcha-key {
Copy link
Contributor

Choose a reason for hiding this comment

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

media stylings should be a the end of css file to overwrite the styling on different devices. Please move the whole media block to the end and I'm sure you can remove the !important tag

@dai-eastgate
Copy link
Contributor Author

@andernath I will check it

@dai-eastgate
Copy link
Contributor Author

dai-eastgate commented Apr 11, 2024

@andernath I have updated the code and here is video demo https://files.fm/u/ddegpwe8v5
Please review it again. Thanks!

andernath
andernath previously approved these changes Apr 15, 2024
Copy link
Contributor

@andernath andernath left a comment

Choose a reason for hiding this comment

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

Thanks @dai-eastgate for refactoring! :)
I just got rid of the last !important tags in CSS, with a more specefic selector

Copy link

Steps to install the approved version:

  1. Download onoffice-4.18.1-31-g742d7c68-please-unpack.zip from https://github.com/onOffice-Web-Org/oo-wp-plugin/actions/runs/8684016907.
  2. Unpack the downloaded file to get another .zip file.
  3. Upload that inner .zip file to WordPress.

@fredericalpers
Copy link
Member

fredericalpers commented Apr 15, 2024

@dai-eastgate On first load the css seems not to be loaded correctly. Only if I open the console and reload it is fixed. Please check and fix this behaviour.

incorrect-styling

@dai-eastgate
Copy link
Contributor Author

On first load the css seems not to be loaded correctly. Only if I open the console and reload it is fixed. Please check and fix this behaviour.

@fredericalpers This bug occurs because the browser cache has not been cleared, so it is saving the CSS in the old branch. When you checkout to a new branch, make sure you clear your browser cache

@fredericalpers fredericalpers merged commit 1740758 into master Apr 15, 2024
3 checks passed
@fredericalpers fredericalpers deleted the 42302-cant-remove-delete-secrets-and-keys branch April 15, 2024 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: dashboard Issue, Pull Request or Discussion related to dashboard / settings page QA Issue or Pull request that is in review UI/UX Issue, Pull Request or Discussion related to UI/UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't remove / delete secrets and keys
3 participants