Skip to content

fix(mapping): stop discarding stale auto-load responses, always clear loading state, keep column-role menu open - #179

Open
benoitcayladbx wants to merge 1 commit into
0.9.0from
fix/mapping-design-panel-query-race-and-ux
Open

benoitcayladbx wants to merge 1 commit into
0.9.0from
fix/mapping-design-panel-query-race-and-ux

Conversation

@benoitcayladbx

Copy link
Copy Markdown
Collaborator

Reopened against 0.9.0 after develop was deleted. GitHub cannot reopen #153 (closed PR + missing base), so this is a successor of #153 with the same commits. Original author: @jeremiaspf. Please rebase onto current 0.9.0 before review — expect conflicts.


What

Three related fixes in the entity/relationship mapping panels (mapping-design.js), rebuilt from scratch against the current develop:

Auto-load race condition. The saved SQL query auto-loads in the background when a panel opens. Its response is now matched against the generation captured when the auto-load was scheduled, not re-read at response time — so it's no longer silently discarded if the panel finishes mounting before the response arrives. Both the entity and relationship panels also stop discarding a response just because the user switched panel/generation in the meantime; the Refresh button and loading spinner are now unconditionally cleared in the completion path, so a slow query can no longer leave the button stuck on "Refreshing..." or the spinner stuck visible. If a previously saved ID column is no longer present in a refreshed result set, the first available column is now auto-selected instead of left unset.

Column-role menu. The per-column menu (assign ID / Label / an attribute to a result column) now stays open across clicks and reopens on the same column after each assignment, instead of closing after every single click — so multiple roles can be assigned across columns in one pass without reopening the menu each time. The existing "Clear" menu item (already present in the markup, added along with the rest of this menu's current design) now actually clears a column's roles — previously the click handler had no branch for data-action="clear" at all, so clicking it silently did nothing.

Why

All three are UX papercuts that make the mapping panels tedious or misleading to use in practice: a stuck "Refreshing..." button after switching panels, a saved ID column silently reset to nothing after a schema change, and a "Clear" button that visibly exists but doesn't work.

Note on scope — rebuilt against current develop

This PR replaces a fix we originally wrote against 0.7.1 and held back, because develop had since reworked this same file substantially (a schema-drift feature: auto-load of live-schema warnings, drift badges, guardedCloseMappingPanel). We rebuilt the fix from scratch directly against the current file rather than porting the old patch, dropping one piece of it that no longer applies: our original fix also renamed a literal DOM id (savePanelBtnmanualSavePanelBtn) shared between the entity and relationship panels' save buttons. That whole button is gone on develop — replaced by auto-save on panel dismiss — so there's nothing left to rename; the two leftover getElementById('savePanelBtn') calls in updateEntityPanelSaveBtn/updateRelPanelSaveBtn are harmless dead code (always null, guarded by if (saveBtn)) and we left them alone rather than touching code unrelated to what this PR fixes.

How to test

  1. Open an entity/relationship panel with a saved SQL query on a slow connection, switch to a different panel before the auto-load response arrives — the Refresh button and loading spinner should not get stuck.
  2. Change the source table's schema so a previously mapped ID column no longer exists, refresh the query — the ID column should auto-select the first available column instead of staying unset.
  3. Open the column-role menu on a result grid, assign ID to one column and an attribute to another without the menu closing in between, then use "Clear" on a column — it should actually clear that column's assigned roles.

… loading state, keep column-role menu open

Three related fixes in the entity/relationship mapping panels:

The saved SQL query auto-loads in the background when a panel opens. Its
response is now matched against the generation captured when it was
scheduled (not re-read at response time), so it is no longer silently
discarded if the panel finished mounting before the response arrived.
Both the entity and relationship panels also stop discarding a response
just because the user switched panel/generation in the meantime — the
Refresh button and loading spinner are now always cleared in the
completion path regardless, so a slow query can no longer leave the
button stuck on "Refreshing..." or the spinner stuck visible. If the
previously saved ID column is no longer present in a refreshed result
set, the first available column is now auto-selected instead of being
left unset.

The per-column role menu (assign ID / Label / an attribute to a result
column) now stays open across clicks and reopens on the same column
after each assignment, instead of closing after every single click —
multiple roles can be assigned to different columns in one pass. The
existing "Clear" menu item (already present in the markup) now actually
clears a column's roles, which it previously did not do at all: the
click handler had no branch for it, so clicking it silently did nothing.

Rebuilt from scratch against the current develop, which has meanwhile
gained a schema-drift feature (auto-load, badges, panel close) in this
same area — this PR only touches the pre-existing race-condition and
menu-interaction issues above, none of it overlaps with schema-drift.
A companion, unrelated bug — a literal DOM id shared between the entity
and relationship panels' save buttons — no longer applies: develop's
redesign replaced the old manual Apply/Save button with auto-save on
panel dismiss, and removed that button from the markup entirely.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants