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

⬆️ Update switch.js #4017

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

josemigallas
Copy link
Contributor

@josemigallas josemigallas commented Feb 18, 2025

TL;DR

This long description is for the sake of describing why this script exists. The PR is pretty straightforward.

Full description

Part of

Switch is a custom component that uses jQuery. It's used both in admin portal and developer portal, but only the admin portal uses jQuery 1.

Admin portal

Used in Application show page, in API Credentials and Referrer filters. The switch shows/hides the "add button" and the "limit warning" alternatively:
Screenshot 2025-02-18 at 09 46 26
Screenshot 2025-02-18 at 09 46 31
Screenshot 2025-02-18 at 09 46 56
Screenshot 2025-02-18 at 09 47 04

Dev Portal

Same usage, different origins. Depending on how long ago the dev portal was created, it uses switch from either:

  • 3scale.js
  • 3scale-v2.js or
  • essential_assets

These are basically copy-pasta files with a collection of scripts from app/assets/javascripts that are used in dev portal:

(function($) {
// ever heard about a toggle() function of jQuery?
$.fn.enableSwitch = function() {
var context = this;
context.find(".disabled_block").fadeOut(function() {
context.find(".enabled_block").fadeIn();
})
}
$.fn.disableSwitch = function() {
var context = this;
context.find(".enabled_block").fadeOut(function() {
context.find(".disabled_block").fadeIn();
})
}
})(jQuery);

⚠️ In order to maintain backwards compatibility with older portals, switch.js has been moved to lib/developer_portal/app/assets/javascripts/switch.js to be loaded in essential_assets.

Screenshot 2025-02-18 at 09 48 50

@josemigallas josemigallas self-assigned this Feb 18, 2025
@josemigallas josemigallas force-pushed the update_jquery_switch_js branch from 6388ba9 to 1098f2e Compare February 18, 2025 11:40
@@ -1,10 +1,9 @@
<% if cinstance.service.backend_version == "2" %>
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is removed because it is redundant?

<% if @cinstance.service.backend_version == "2" %>
<%= render 'applications/applications/api_credentials', :cinstance => @cinstance %>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct

@josemigallas josemigallas force-pushed the update_jquery_switch_js branch from 1098f2e to 8395d09 Compare February 19, 2025 09:06
lvillen
lvillen previously approved these changes Feb 20, 2025
@josemigallas josemigallas force-pushed the update_jquery_switch_js branch from e7d6d40 to 3215c96 Compare February 20, 2025 14:41
Copy link
Contributor

@jlledom jlledom left a comment

Choose a reason for hiding this comment

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

I tried the the key section in the Application screen:

  • Removing keys works fine
  • Adding a key , random or manually, shows a JS error, but the key is added correctly. You need to refresh the screen to see it.
  • Referrer filter works fine

Comment on lines +122 to +124
# FIXME: at some point in the past this feature broke
# Then they should see "At most 5 keys are allowed."
# And there should not be a button to "Create new key" within the application keys
Copy link
Contributor

Choose a reason for hiding this comment

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

Then, where did you get this cukes from? Were they somewhere else?

Comment on lines +139 to +140
# FIXME: at some point in the past this feature broke
# And there should be a button to "Create new key" within the application keys
Copy link
Contributor

Choose a reason for hiding this comment

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

Can cukes be fixed?

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