-
Notifications
You must be signed in to change notification settings - Fork 13
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
Refactor JavaScript #58
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
clhenrick
force-pushed
the
js-refactor-webpack
branch
2 times, most recently
from
September 8, 2020 02:43
b6970be
to
88e20d3
Compare
clhenrick
force-pushed
the
js-refactor-webpack
branch
from
September 24, 2020 03:58
31352fe
to
9c7a7db
Compare
clhenrick
force-pushed
the
js-refactor-webpack
branch
from
September 24, 2020 16:23
a201ce1
to
c810afb
Compare
uses the geocoding endpoint provided by NYC Planning Labs see: https://labs-geosearch-docs.netlify.app/docs/#autocomplete
listens to state.addressGeocode updates dispatches async action addressGeocodeFetch on input event
- rm now unused borough dropdown styles - center address search text input - rm dead code
- made it an actual button element - moved it to be within the form element
clhenrick
commented
Nov 26, 2020
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.
Reviewed up to navigation.js
cleaned up, removed unused code, & added more tests
- made sure html element is validated using jest .not.toBeNull() - made sure instance is created in beforeEach - made sure jest.clearAllMocks() is called in afterEach
log bbl instead of street address for match, rm getters for searchResult props
This was referenced Dec 1, 2020
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JS Refactor To Dos
This doc outlines tasks for refactoring the JavaScript in amirentstabilized.com
Code Refactor
app/js/app
directoryComponents
Create Component classes for each interactive element to isolate its logic, styles, and DOM element(s).
State Management
redux-logger
in a "debug" mode for debugging builds when necessaryFrontend Build System Eval
Code Quality Improvement
(or keep using JS Hint?)Static Assets
data/
directory tolocales/
[page-name]-[lang-code].json
file naming convention3rd Party Deps
could replace slide scroll animation withelement.scrollIntoView
also see smooth scroll behavior polyfill) [NOTE this doesn't seem to work for my use case with Safari)bower_components
(think this is justcartodb.js
?)cartodb.js
dep)cartodb.js
aja.js
addthis.js
(social media sharing widget)atc.js
(Add to Calendar)handlebars.js
add / usealpine.js
to refactor & reduce the amount of JS neededMap
Other
move domain name to Netlify DNS(need to do this after merging this PR to master)inert
html property (video demo) (requires a polyfill for some browsers)Use NYC Planning Labs Geocoder for address search
The current address geocoding API is an earlier version that requires the street address and borough to be passed as separate params. The newer version supports autosuggest which is a better UX.
Track Bugs Introduced by JS Refactor
SearchResultMap
's size (maybe a reference is being retained to the old DOM element that was destroyed when the content re-renders?)when slide index > 0 then page is translated, it starts at the first slide but theslides.curIndex
property persists so that whenGoToNextSlide
is dispatched the slides jump ahead multiple slides.Misc Clean Up
state.slides.canAdvance
boolean?