Fredhase/codeclimate refactor onboarding initializers 3739 #5809
Conversation
| const drawerSliders = [ | ||
| { selector: 'sidebar-bg-left', swipeState: 'middle', side: 'left', view: 'outOfView' }, | ||
| { selector: 'sidebar-bg-right', swipeState: 'middle', side: 'right', view: 'outOfView' }, | ||
| { selector: 'on-page-nav-butt-left', swipeState: 'left', side: 'left', view: 'intoView' }, | ||
| { selector: 'on-page-nav-butt-right', swipeState: 'right', side: 'right', view: 'intoView' }, | ||
| ]; |
There was a problem hiding this comment.
This could be kept outside the function.
| } | ||
| } | ||
| InstantClick.on('change', function() { | ||
| document.getElementsByTagName('body')[0].classList.remove('modal-open'); |
There was a problem hiding this comment.
This classList.remove('modal-open') seems to be missed in the new code.
| } else { | ||
| swipeState = 'middle'; | ||
| slideSidebar('left', 'outOfView'); |
There was a problem hiding this comment.
you can avoid using else at all
if (!document.getElementById('on-page-nav-controls')) {
return;
}
if (swipeState == 'middle') {
swipeState = 'right';
return slideSidebar('right', 'intoView');
}
swipeState = 'middle';
slideSidebar('left', 'outOfView');| @@ -125,42 +138,10 @@ class ClosingSlide extends Component { | |||
| </div> | |||
| ); | |||
| } | |||
| return ( | |||
| return( | |||
| } | ||
|
|
||
| whatsNext() { | ||
| const properties = [ |
There was a problem hiding this comment.
please, rename this to something like whatsNextLinks,
or just return the array (see next comment)
| return( | ||
| properties.map((element, index) => | ||
| <a key={ index } href={ element.href } data-no-instant={(index === 0) ? true : null }> | ||
| { element.text } | ||
| <p className="whatnext-emoji"> | ||
| <span role="img" aria-label="tada"> | ||
| { element.emoji } | ||
| </span> | ||
| </p> | ||
| </a> | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Doesn't add much to have a method return a small piece of component here.
As a suggestion:
- make
whatsNext()method return just the array of objects. .mapit inrender()method
|
@paulodiovani |
|
@nickytonline |
No worries @fredhase. And no pressure to get this done. If you need some assistance with branches and git, happy to help. This post from one of our community members may be of help to you, https://dev.to/unseenwizzard/learn-git-concepts-not-commands-4gjc |
|
Closing this PR for some branche mistakes that caused conflict |
What type of PR is this? (check all applicable)
Description
Refactoring codes in app/assets/javascripts/initializers (DrawerSliders and SwipeGestures). Some minor refactoring and some repetitive blocks are now a function.
In app/javascript/onboarding/components just made minor refactoring.
Added to documentation?