PurdueMarketingAndMedia / purdueTemplates-2015

Included in this project are the Purdue University templates developed by the Office of Marketing and Media in 2015.
14 stars 7 forks source link

toggle.js bug? #106

Open jobokai opened 4 years ago

jobokai commented 4 years ago

https://github.com/PurdueMarketingAndMedia/purdueTemplates-2015/blob/master/src/js/mm/global/toggle.js#L128

It seems like there is a bug on this line where dropdown is not actually defined, unless I'm missing it's declaration somewhere. I'm noticing in my builds that I'm not actually getting the dropdown for Find Info For working.

jobokai commented 4 years ago

Just did a fresh pull and the goldbar still seems to be bugged for me. I'm not sure if I'm missing something right in front of my face 😅

Yanhuazong commented 4 years ago

Thank you for reporting this issue. The dropdown on that line should be menu. The other dropdowns are also not properly deselected when the Find Info button clicked. I guess the correct code for toggling the Find Info button should be:

case checkElement(clicked, '.header__goldBar__findInfoFor button'): // specifically find info for button
            const menu = document.querySelector('#findInfoFor')
            const currDisplayVal = getCurrDisplay(menu)
            let selectedObj = getCurrSelected()
            let outSelectedObj = selectedObj.outerSelected
            let inSelectedObj = selectedObj.innerSelected
            let siSelectedObj = selectedObj.sideSelected
            const allDropdowns = [...document.querySelectorAll('.header__mainNav--dropdownOuter'), ...document.querySelectorAll('.header__mainNav--dropdownInner'), document.querySelector('#searchDropdown'),...document.querySelectorAll('.dropdown-content')]
            allDropdowns.map((checkDropdown) => {
                if (checkDropdown !== menu) {
                    hide(checkDropdown)
                    deselect(outSelectedObj)
                    deselect(inSelectedObj)
                    deselect(siSelectedObj)
                }
            })
            if (currDisplayVal !== 'none') {
                hide(menu)
            } else {
                show(menu)
            }
            break

We will update the repo soon. Are you still having issue with the goldbar? What kind of issue are you having? Thanks!

jobokai commented 4 years ago

So I have a rather extensive update of the toggle.js file for both linting purposes and for fixing a variety of bugs that I was encountering. I think the biggest thing I ran into was in the switch(https://github.com/PurdueMarketingAndMedia/purdueTemplates-2015/blob/master/src/js/mm/global/toggle.js#L24) the variables that were being set, carried over into other cases. This is a bit antipattern as each case should be unique in and of itself. https://eslint.org/docs/rules/no-case-declarations

jobokai commented 4 years ago

107 I opened a PR (I think I did it correctly) that should address the changes I made. Let me know what you all think.