-
Notifications
You must be signed in to change notification settings - Fork 20
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
Split ExtendedData type into base data and additional data #546
base: main
Are you sure you want to change the base?
Conversation
…search, fix initial no search results glitch
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Ohayou! プルリクエストありがとうございます。いくつかの試験に集中する必要があるため、今週後半にこれをレビューする予定です。 |
どうもありがとうございます 🙏 |
|
pascal case with underscores - joining it together is rather unreadable |
I would call it: |
see wouldn't |
I think readability is subjective - maybe there is research done that can prove me wrong. I think the reason for pure pascal case can be reduced to readability, and consistency with existing convention. Also, with these underscores, you are introducing another point of ambiguity. What is the strategy for determining where underscores go? I suppose a satisfactory way to end this debate would be to find a scientific paper that proves that one method yields higher accuracy than the other. I wasn't able to find any papers (much less any examples) that mix together snake case and pascal case, though. |
This was closest I could find of any sort. States that underscores are more readable. I don't think it combines them though. camelCase is close enough to PascalCase.: https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=5521745 |
interesting! It would probably be unwise to make everything python underscore case though lol |
* style: smoother loading experience * chore: style cleanup * chore: remove existing search param * chore: fix lint * fix: disable animation when user uses search * fix: a bit smoother title reveal * fix: lint * chore: I think this is a fair solution * fix: css variable delay refactor and card hover fix * chore: fix lint * chore: fix lint * style: add motion blur * fix: less blur
I guess the styling changes will also be merged here lol. |
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.
Thank you for the PR. There are two issues.
-
assertThatLocationsAndExtraLocationDataAreInSync is really long. Can you please shorten in. Maybe assertLocationDataInSync.
-
There is inconsistency in naming. That is, (IReadOnlyLocation_FromAPI_PreProcessed and IReadOnlyLocation_FromAPI_PostProcessed) vs. (IReadOnlyLocationExtraDataMap and IReadOnlyLocationExtraData). We also should choose one standard as Gabe suggested.
Animation side of changes is good as I mentioned in the other PR.
Ohayou!
This PR splits API data into its original
locations
data and any extra status data (the stuff that gets updated every second) into a map fromconceptId
to data. We then rejoin the two things insideListPage
. Types were updated accordingly. (hopefully to better names this time around...)Why?
Bug fixes
locations
was populated). I changed everything touseMemo
and React now updates everything in one pass-throughThe bug:
Screen.Recording.2025-02-01.at.8.08.40.PM.mov
Description
Type of change
How Has This Been Tested?
Test Configuration:
Checklist: