-
Notifications
You must be signed in to change notification settings - Fork 4
ENG-1454: Base getter() task #828
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
Open
sid597
wants to merge
51
commits into
main
Choose a base branch
from
migration-block-init-staging-branch
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
51 commits
Select commit
Hold shift + click to select a range
e7114b4
ENG-1465: Add "Use new settings store" feature flag (#811)
sid597 c324bb6
ENG-1455: Dual-read from old-system settings and from blockprops (#812)
sid597 700b590
ENG-1472: Refactor BlockPropSettingPanels to add accessor-backed defa…
sid597 b88f56a
Eng-1473 port non boolean personal setting consumers (#823)
sid597 4c908aa
ENG-1467: Port global setting consumer reads (→ getGlobalSetting) (#824)
sid597 47411c2
ENG-1456: Migrate personal boolean flag consumers (getSetting → getPe…
sid597 0bc1d22
ENG-1479: Port suggestive mode settings to dual-read (#845)
sid597 3b4c80a
ENG-1468: Port node and relations tree based consumers (#825)
sid597 f6cc8c2
ENG-1478: Port left sidebar to dual-readers + reactivity
sid597 5155a2c
ENG-1478: DRY merge helpers, real dual-read in settings panels, emitt…
sid597 686fd05
ENG-1478: refresh config tree before emitter-driven rebuild
sid597 eea529a
ENG-1478: extract emitter key constants, fix stale TODO
sid597 5925be2
ENG-1478: fix legacy adapter child.uid → child.text, fix settingKeys …
sid597 ee8bf56
ENG-1478: skip reload prompt when new settings store is active
sid597 ade239b
ENG-1478: guard getGlobalSetting/getPersonalSetting against empty keys
sid597 b7d26df
ENG-1469: Refactor getDiscourseRelations and getDiscourseNodes to rea…
sid597 a1ce922
Fix: use backedBy 'user' for initial discourse nodes in block props
sid597 915c0eb
Hardcode backedBy 'user' in toDiscourseNode to match legacy behavior
sid597 d682a86
ENG-1484: reactive settings for triggers and suggestive mode overlay
sid597 d280bfd
ENG-1484: use nullish coalescing for trigger fallbacks to match init …
sid597 2fdb3de
ENG-1484: reactive suggestive mode flag, gate reload prompts on new s…
sid597 bc261e0
ENG-1484: broad selector for suggestive overlay cleanup to match legacy
sid597 0ee94fa
ENG-1484: fix prettier formatting (root config)
sid597 dc1d46a
ENG-1484: drop reload prompts — reactivity is unconditional via dual-…
sid597 46fa9bc
Merge pull request #844 from DiscourseGraphs/eng-1478-port-both-globa…
sid597 6606ffa
Merge pull request #853 from DiscourseGraphs/eng-1469-refactor-getdis…
sid597 b5257d7
ENG-1499: Replace raw string accessor paths with shared constants fro…
sid597 5a462ce
Fix missed keyImageOption getter to use shared constants
sid597 fc58bf3
Merge pull request #875 from DiscourseGraphs/eng-1499-convert-legacy-…
sid597 cd16a66
ENG-1519: Add legacy-to-blockprops migration, remove backedBy from sc…
sid597 b97def3
Fix shouldWrite skipping after reconciliation, warn on missing person…
sid597 14e72c9
Add migration telemetry for legacy parse failures
sid597 def877f
Use backend query for node migration
sid597 cd2fa4c
ENG-735: Remove inline settings UI from config pages
sid597 f3cae70
Use Object.values instead of unused destructured key
sid597 991a7c9
ENG-1503: Replace getFormattedConfigTree consumers with direct helper…
sid597 3b67eec
ENG-1484: extract constants, remove dead defaults per review
sid597 bdbbbc4
Fix useConfig to call buildConfig() for dual-read merge
sid597 7f51464
merge migration-block-init-staging-branch
sid597 c44b338
prettier
sid597 cbf7345
Merge pull request #860 from DiscourseGraphs/eng-1503-remove-remnants…
sid597 028bec6
Merge migration-block-init-staging-branch into eng-735
sid597 6e28926
Fix import path for DISCOURSE_CONFIG_PAGE_TITLE
sid597 29f99fc
Merge branch 'migration-block-init-staging-branch' into eng-1519-lega…
sid597 f62b3f8
fix review
sid597 a1f1dc0
Merge pull request #876 from DiscourseGraphs/eng-1519-legacy-to-block…
sid597 8b23c58
Merge pull request #878 from DiscourseGraphs/eng-735-remove-ui-from-s…
sid597 fd332e9
Merge remote-tracking branch 'origin/migration-block-init-staging-bra…
sid597 4877527
Merge pull request #861 from DiscourseGraphs/eng-1484-reactive-settin…
sid597 19f90c0
Merge origin/main into migration-block-init-staging-branch
sid597 26fef42
Merge origin/main (ENG-1403, ENG-1508) into migration-block-init-stag…
sid597 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
🟡 Inverted fold/unfold logic:
isOpenstate and block CRUD are backwardsIn
toggleFoldedState, whenisOpenistrue(section is currently open/expanded), the code callssetIsOpen(false)and deletes the "Folded" block — meaning it unfolds by removing the folded marker. But whenisOpenisfalse, it callssetIsOpen(true)and creates a "Folded" block. This means theisOpenReact state tracks whether the section is collapsed (folded), not whether it is open. ThenewFoldedvalue atLeftSidebarView.tsx:130is computed as!isOpen, so whenisOpen=true(collapsed/folded),newFolded=false— and this is the value written to the new settings store. However, the old-system block CRUD on lines 132-148 does the opposite: whenisOpen=trueit deletes the Folded block (setting folded=false), and whenisOpen=falseit creates the Folded block (setting folded=true). So both systems agree on the final folded value.The actual bug is that the
newFoldedvalue written to the new settings store at lines 150-166 is!isOpen, but the old-system block operations establish thatisOpen=truemeans "currently folded" (since the Folded block exists). When the section is currently folded (isOpen=true),newFolded = !true = false→ correct (we're unfolding). When the section is currently expanded (isOpen=false),newFolded = !false = true→ correct (we're folding). So the value is actually correct.However, looking at
PersonalSectionItemat line 223,isOpenis initialized from!!section.settings?.folded.value, meaningisOpen=truewhenfolded=true. This meansisOpenactually represents "is folded", not "is open". TheCollapse isOpen={isOpen}at line 262 will then show content whenfolded=trueand hide content whenfolded=false— which is the opposite of expected behavior. The section is shown expanded when the folded flag is true, and collapsed when it's false.(Refers to lines 223-224)
Was this helpful? React with 👍 or 👎 to provide feedback.