-
Notifications
You must be signed in to change notification settings - Fork 229
Mahdiyeh/ Fix: guide on 4th step #6753
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/deriv/binary-static/7ougGqkVSGfMzBNc9oTkqhqErxfF |
|
A production App ID was automatically generated for this PR. (log)
Click here to copy & paste above information. |
src/javascript/app/common/guide.js
Outdated
| const setEvents = () => { | ||
| $(`${opt.guideBtnID} strong`).click(() => { | ||
| const enjoyhint_instance = new EnjoyHint({}); | ||
| const contractList = $(opt.contractList); |
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.
Can we use snake_case for these constants? contract_list and close_confirmation
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.
since guideBtnID is in the camelCase format, i keep it the same
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 think it's the old practice which we don't use anymore. Maybe you can also change guideBtnID to guide_btn_id?
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.
My suggestion would be to make all three consistent like guide_btn_id, contract_list_id, and close_confirmation_id. Do you think this would be better?
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.
yeah, right, will handle that, thanks
a3a476e to
ef193b8
Compare
ef193b8 to
e979c51
Compare
|
Kudos, SonarCloud Quality Gate passed!
|








No description provided.