anonymous user should be able to see cpp/java but cannot select it #7955
Conversation
WalkthroughThis set of changes replaces the custom "fancySelect" dropdown implementation with the Choices.js library for language and code format selection in the play UI. It removes the legacy jQuery-based plugin and its associated CSS, updates the JavaScript logic to instantiate and manage Choices.js instances, and adjusts SASS and Pug templates to match the new structure and styling. The package.json is updated to include Choices.js as a dependency, and custom SASS is added for the new dropdown style. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChangeLanguageTab
participant Choices.js
User->>ChangeLanguageTab: Interacts with language/code format dropdown
ChangeLanguageTab->>Choices.js: Initializes or destroys Choices instance as needed
Choices.js-->>ChangeLanguageTab: Renders enhanced dropdown UI
User->>Choices.js: Selects a language/format
Choices.js-->>ChangeLanguageTab: Triggers change event
ChangeLanguageTab->>ChangeLanguageTab: Handles selection logic and updates state
ChangeLanguageTab->>Choices.js: (If needed) Rebuilds dropdown
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/views/play/common/ChangeLanguageTab.js (1)
189-264: Consider consistent naming convention for Choices instancesThe codebase uses inconsistent naming conventions between
languageChoicesandcodeformatChoices. Consider standardizing to eitherlanguageChoices/codeFormatChoicesorlanguagechoices/codeformatchoices.- this.languageChoices = null - this.codeformatChoices = null + this.languageChoices = null + this.codeFormatChoices = nullUpdate all references accordingly throughout the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
app/styles/play/common/change_language_tab.sass(1 hunks)app/styles/play/modal/play-heroes-modal.sass(3 hunks)app/templates/play/common/change-language-tab.pug(1 hunks)app/vendor.js(0 hunks)app/views/play/common/ChangeLanguageTab.js(4 hunks)package.json(1 hunks)vendor/scripts/fancy_select.js(0 hunks)vendor/styles/fancy_select.css(0 hunks)
💤 Files with no reviewable changes (3)
- app/vendor.js
- vendor/styles/fancy_select.css
- vendor/scripts/fancy_select.js
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ESLint CI
- GitHub Check: Node.js CI (18.10.0)
🔇 Additional comments (13)
package.json (1)
111-111: LGTM: Added Choices.js dependency for the dropdown replacementThe addition of the
choices.jsdependency (v11.1.0) correctly supports the replacement of the previously used customfancySelectplugin with a more modern dropdown solution.app/styles/play/common/change_language_tab.sass (1)
1-28: Styles look good for the new Choices.js dropdown implementationThe new stylesheet properly imports base styles from the library and provides custom styling for the dropdown elements. The styling maintains consistent colors with the application's design system (#2B8686 for text, #abc for hover state).
app/templates/play/common/change-language-tab.pug (1)
2-5: LGTM: Updated template structure for Choices.js compatibilityThe template structure has been appropriately modified to work with Choices.js, wrapping the select in a container with the new class name and updating option elements accordingly.
app/styles/play/modal/play-heroes-modal.sass (3)
530-541: LGTM: Updated selectors for RTL supportThe selectors have been properly updated from
.fancy-selectto.my-fancy-selectandlito.itemto match the new Choices.js structure while maintaining RTL (right-to-left) language support.
542-567: LGTM: Updated main dropdown stylingThe main dropdown styling has been properly updated to use
.my-fancy-selectinstead of.fancy-select, and the addition ofborder-radius: 4pxgives the trigger a more polished appearance.
579-595: LGTM: Updated language option stylingThe language option styling has been properly updated to use
.iteminstead ofliselectors, with the addition ofmargin-bottom: -1pxto improve vertical alignment of the options.app/views/play/common/ChangeLanguageTab.js (7)
13-17: LGTM: Added required imports for Choices.jsThe new import statements correctly add the Choices.js library and ensure the associated styles are included.
189-203: Good implementation of Choices.js for code formatsThe implementation correctly initializes Choices.js for code formats with appropriate configuration. The mobile check is a good optimization to avoid unnecessarily initializing the library on mobile devices.
214-228: Good implementation of Choices.js for code languagesThe implementation correctly initializes Choices.js for language selection with appropriate configuration, matching the pattern used for code formats.
229-236: Consistent implementation of language option blurbsThe code for appending localized blurbs to each dropdown option's text is maintained, preventing duplication when called multiple times.
242-244: Good addition of rebuilding the language dropdownThe addition of
this.buildCodeLanguages()ensures the dropdown is properly reconstructed after changing the language.
256-257: Good addition of rebuilding the format dropdownThe addition of
this.buildCodeFormats()ensures the dropdown is properly reconstructed after changing the code format.
204-211:❓ Verification inconclusive
Verify DOM traversal works with Choices.js structure
The DOM traversal has been updated to accommodate Choices.js structure, but let's ensure it works correctly.
🏁 Script executed:
#!/bin/bash # Search for any other instances where DOM traversal might need similar adjustments rg -n "find\(.*\.options" --type js app/Length of output: 1147
Confirm DOM traversal in ChangeLanguageTab.js matches Choices.js markup
I’ve searched the codebase and only two instances of the old
.options .itemtraversal remain (both in ChangeLanguageTab.js). Please verify that these still target the correct elements in the new Choices.js dropdown and update the selectors if the HTML structure or class names have changed.• app/views/play/common/ChangeLanguageTab.js
– Line 204:$select.parent().parent().find('.options .item')
– Line 229: identical traversal in the second handlerTest both in the browser and adjust (e.g. to
.choices__list--dropdown .choices__item) if necessary.
| if (this.languageChoices) { | ||
| this.languageChoices.destroy() | ||
| } | ||
| this.languageChoices = new Choices($select[0], { |
There was a problem hiding this comment.
language and codeformat use the same object as second parameter, DRY code?
There was a problem hiding this comment.
ok. yes we only use fancy-select here lol
fancy-select doesn't show the disabled item.
so get rid of this old package and use alternative chioces.js
fix ENG-1729
Summary by CodeRabbit
New Features
Refactor
Chores