Skip to content

added functionality to share window data in URL #2814

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 21, 2025
Merged

added functionality to share window data in URL #2814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions packages/altair-app/src/app/modules/altair/effects/account.effect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,20 +281,6 @@ export class AccountEffects {
{ dispatch: false }
);

onAccountCheckedInit$ = createEffect(
() => {
return this.actions$.pipe(
ofType(accountActions.ACCOUNT_IS_LOGGED_IN),
switchMap((action) => {
// check for shared links
this.sharingService.checkForShareUrl();
return EMPTY;
})
);
},
{ dispatch: false }
);

constructor(
private actions$: Actions,
private store: Store<RootState>,
Expand Down
23 changes: 20 additions & 3 deletions packages/altair-app/src/app/modules/altair/effects/windows.effect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { downloadJson, openFile } from '../utils';
import { RootState } from 'altair-graphql-core/build/types/state/state.interfaces';
import { debug } from '../utils/logger';
import { windowHasUnsavedChanges } from '../store';
import { APP_INIT_ACTION } from '../store/action';
import { SharingService } from '../services';

@Injectable()
export class WindowsEffects {
Expand Down Expand Up @@ -218,9 +220,24 @@ export class WindowsEffects {
{ dispatch: false }
);

checkShareUrls$ = createEffect(
() => {
return this.actions$.pipe(
ofType(APP_INIT_ACTION),
switchMap(() => {
// check for shared links
this.sharingService.checkForShareUrl();
return EMPTY;
})
);
},
{ dispatch: false }
);

constructor(
private actions$: Actions,
private store: Store<RootState>,
private windowService: WindowService
private readonly actions$: Actions,
private readonly store: Store<RootState>,
private readonly windowService: WindowService,
private readonly sharingService: SharingService
) {}
}
84 changes: 62 additions & 22 deletions packages/altair-app/src/app/modules/altair/services/sharing/sharing.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,22 @@ import { AccountService } from '../account/account.service';
import { copyToClipboard } from '../../utils';
import { consumeQueryParam } from '../../utils/url';

interface ShareDetails {
// ?q=<queryId>
interface SharedRemoteQuery {
type: 'remote-query';
queryId: string;
}

// ?query=<query>&variables=<variables>&endpoint=<endpoint>
interface SharedWindowData {
type: 'window-data';
endpoint: string | undefined;
query: string;
variables: string | undefined;
}

type SharedUrlInfo = SharedRemoteQuery | SharedWindowData;

@Injectable({
providedIn: 'root',
})
Expand All @@ -30,43 +43,70 @@ export class SharingService {
if (!shareDetails) {
return;
}
this.accountService.observeUser().subscribe((user) => {
if (!user) {
return;
}
this.handleShareDetails(shareDetails);
});

this.handleShareDetails(shareDetails);
Comment on lines 43 to +47
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The original code had an account check here. Is it intentional that this check is removed? If so, please add a comment explaining why it's no longer needed. If not, please add the account check back in to prevent issues when a user is not logged in.

}

copyQueryShareUrl(queryId: string) {
copyToClipboard(this.apiService.getQueryShareUrl(queryId));
this.notifyService.info(`Copied share url to clipboard`);
}

private getShareDetailsFromUrl(url: string) {
private getShareDetailsFromUrl(url: string): SharedUrlInfo | undefined {
const queryId = consumeQueryParam('q', url);
if (!queryId) {
// no shared link
return;
if (queryId) {
// shared remote query
return { type: 'remote-query', queryId };
}
const query = consumeQueryParam('query', url);
if (query) {
// shared window data
return {
type: 'window-data',
query,
variables: consumeQueryParam('variables', url),
endpoint: consumeQueryParam('endpoint', url),
};
}

return { queryId };
return;
}
Comment on lines +55 to 73
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Bug: removed query params can re‑appear because a stale url string is reused

consumeQueryParam() mutates window.location.href via history.replaceState, but here you always pass the original url string. On the second/third call (variables, endpoint) the parsed URL still contains the previously removed query param, so it gets written back, effectively resurrecting it.

Fix: after the first extraction, let subsequent calls read from the freshly mutated window.location.href by omitting the second argument (or updating the local url variable).

-    const queryId = consumeQueryParam('q', url);
+    const queryId = consumeQueryParam('q', url);
...
-    const query = consumeQueryParam('query', url);
+    const query = consumeQueryParam('query');
...
-        variables: consumeQueryParam('variables', url),
-        endpoint: consumeQueryParam('endpoint', url),
+        variables: consumeQueryParam('variables'),
+        endpoint: consumeQueryParam('endpoint'),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private getShareDetailsFromUrl(url: string): SharedUrlInfo | undefined {
const queryId = consumeQueryParam('q', url);
if (!queryId) {
// no shared link
return;
if (queryId) {
// shared remote query
return { type: 'remote-query', queryId };
}
const query = consumeQueryParam('query', url);
if (query) {
// shared window data
return {
type: 'window-data',
query,
variables: consumeQueryParam('variables', url),
endpoint: consumeQueryParam('endpoint', url),
};
}
return { queryId };
return;
}
private getShareDetailsFromUrl(url: string): SharedUrlInfo | undefined {
const queryId = consumeQueryParam('q', url);
if (queryId) {
// shared remote query
return { type: 'remote-query', queryId };
}
const query = consumeQueryParam('query');
if (query) {
// shared window data
return {
type: 'window-data',
query,
variables: consumeQueryParam('variables'),
endpoint: consumeQueryParam('endpoint'),
};
}
return;
}


private async handleShareDetails(shareDetails: ShareDetails) {
private async handleShareDetails(shareDetails: SharedUrlInfo) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider extracting the logic for each share type into separate helper methods to reduce nesting and improve readability of the code in the handleShareDetails function.

Consider extracting the logic for each share type into separate helper methods. This would isolate the asynchronous flow and reduce the nesting within the switch-case. For example:

private async handleWindowData(details: SharedWindowData) {
  const { query, variables, endpoint } = details;
  return this.windowService.importWindowData({
    version: 1,
    type: 'window',
    query,
    variables: variables ?? '{}',
    apiUrl: endpoint ?? '',
    windowName: 'From url',
    headers: [],
    subscriptionUrl: '',
  });
}

private async handleRemoteQuery(details: SharedRemoteQuery) {
  const user = await this.accountService.getUser();
  if (!user) {
    return;
  }
  const res = await this.apiService.getQuery(details.queryId);
  if (!res) {
    throw new Error('Query not found');
  }
  return this.windowService.loadQueryFromCollection(
    res.query,
    res.collectionId,
    res.query.id ?? details.queryId
  );
}

private async handleShareDetails(details: SharedUrlInfo) {
  try {
    switch (details.type) {
      case 'window-data':
        return this.handleWindowData(details);
      case 'remote-query':
        return this.handleRemoteQuery(details);
    }
  } catch (err) {
    debug.error(err);
    this.notifyService.error(`Error loading shared details`);
  }
}

This refactoring keeps functionality intact while reducing complexity in the main handler.

try {
const res = await this.apiService.getQuery(shareDetails.queryId);
if (!res) {
throw new Error('Query not found');
switch (shareDetails.type) {
case 'window-data': {
const { query, variables, endpoint } = shareDetails;
return this.windowService.importWindowData({
version: 1,
type: 'window',
query,
variables: variables ?? '{}',
apiUrl: endpoint ?? '',
windowName: 'From url',
headers: [],
subscriptionUrl: '',
});
}
Comment on lines +77 to +90
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Consider adding specific error handling for the window-data case. For example, if windowService.importWindowData fails, provide a specific error message to the user.

case 'remote-query': {
const user = await this.accountService.getUser();
if (!user) {
return;
}
const res = await this.apiService.getQuery(shareDetails.queryId);
if (!res) {
throw new Error('Query not found');
}
return this.windowService.loadQueryFromCollection(
Comment on lines +92 to +100
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Silent no‑login exit may confuse users

If a remote‑query link is opened while the user is not logged in, the function just returns. Consider surfacing a notification:

-          if (!user) {
-            return;
-          }
+          if (!user) {
+            this.notifyService.warn('Log in to view the shared query');
+            return;
+          }

This provides feedback instead of failing silently.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const user = await this.accountService.getUser();
if (!user) {
return;
}
const res = await this.apiService.getQuery(shareDetails.queryId);
if (!res) {
throw new Error('Query not found');
}
return this.windowService.loadQueryFromCollection(
const user = await this.accountService.getUser();
if (!user) {
this.notifyService.warn('Log in to view the shared query');
return;
}
const res = await this.apiService.getQuery(shareDetails.queryId);
if (!res) {
throw new Error('Query not found');
}
return this.windowService.loadQueryFromCollection(

res.query,
res.collectionId,
res.query.id ?? shareDetails.queryId
);
}
Comment on lines +91 to +105
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Consider adding specific error handling for the remote-query case. For example, if accountService.getUser or apiService.getQuery fails, provide a specific error message to the user.

}
Comment on lines +75 to 106
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The handleShareDetails function has a try-catch block that catches any error and displays a generic error message. It would be more helpful to provide specific error messages based on the type of error encountered. For example, if the query is not found, display a message indicating that the query does not exist or the user does not have permission to access it.

await this.windowService.loadQueryFromCollection(
res.query,
res.collectionId,
res.query.id ?? shareDetails.queryId
);
} catch (err) {
debug.error(err);
this.notifyService.error(`Error loading shared query`);
this.notifyService.error(`Error loading shared details`);
}
}
}
5 changes: 5 additions & 0 deletions packages/altair-docs/.vitepress/config.mts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { defineConfig } from 'vitepress';
import { getConfig } from './plugins/sidebar-generation';
import coreApiSidebar from '../api/core/typedoc-sidebar.json';
import { dynamicFiles } from './plugins/dynamic-files';
import { toString } from 'hast-util-to-string';
import { openInAltairShikiPlugin } from './plugins/open-in-altair-shiki';

const { sidebar: retrievedSidebar } = getConfig({
filter: (meta) => meta.sidebar !== false,
Expand Down Expand Up @@ -139,4 +141,7 @@ export default defineConfig({
vite: {
plugins: [dynamicFiles()],
},
markdown: {
codeTransformers: [openInAltairShikiPlugin()],
},
});
37 changes: 37 additions & 0 deletions packages/altair-docs/.vitepress/plugins/open-in-altair-shiki.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { toString } from 'hast-util-to-string';
import { ShikijiTransformer } from 'shikiji';
export function openInAltairShikiPlugin(): ShikijiTransformer {
return {
code(node) {
if (this.options.lang === 'graphql') {
node.children.push({
type: 'element',
tagName: 'button',
properties: {
type: 'button',
data: toString(node),
title: 'Open in Altair Web',
'aria-label': 'Open in Altair Web',
class: 'open-in-altair-btn',
'data-name': 'open-in-altair-button',
onclick: /* javascript */ `
const url = new window.URL('https://web.altairgraphql.dev/');
const params = new URLSearchParams();
params.set('query', this.attributes.data.value);
params.set('variables', JSON.stringify({}));
url.search = params.toString();

window.open(url.toString(), '_blank');
`.trim(),
},
children: [
{
type: 'text',
value: 'Open in Altair Web',
},
],
});
}
},
};
}
21 changes: 21 additions & 0 deletions packages/altair-docs/.vitepress/theme/custom.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,24 @@ p {
.medium-zoom-image--opened {
z-index: 31;
}

.open-in-altair-btn {
position: absolute;
bottom: 10px;
right: 10px;
font-size: 12px;
font-weight: 500;
font-family: var(--vp-font-family-base);
z-index: var(--vp-z-index-local-nav);
background-color: var(--vp-code-copy-code-bg);
color: var(--vp-code-copy-code-active-text);
border: 1px solid var(--vp-code-copy-code-border-color);
color: white;
padding: 5px 10px;
border-radius: 5px;
transition: all 0.3s ease;
}
.open-in-altair-btn:hover {
background-color: var(--vp-code-copy-code-hover-bg);
border: 1px solid var(--vp-code-copy-code-hover-border-color);
}
Comment on lines +49 to +68
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify the button works correctly with dark/light theme modes

The button styling uses theme variables for colors which is good, but the explicit color: white might not work well in all themes, especially light themes. Consider using the theme variable consistently instead of hardcoding the text color.

🏁 Script executed:

#!/bin/bash
# Check if there are any other buttons or UI elements that handle theme switching correctly
grep -r "color:" --include="*.css" packages/altair-docs/.vitepress/theme/ | grep -v "white" | grep -B 2 -A 2 "var(--vp"

Length of output: 13582

Remove hardcoded white text color in .open-in-altair-btn

The button already sets

color: var(--vp-code-copy-code-active-text);

so the subsequent

color: white;

override prevents it from adapting in light mode. Please remove the hardcoded white and rely solely on the theme variable.

• File: packages/altair-docs/.vitepress/theme/custom.css
• Lines ~53–56: drop the color: white; override

Suggested diff:

 .open-in-altair-btn {
   position: absolute;
   bottom: 10px;
   right: 10px;
   font-size: 12px;
   font-weight: 500;
   font-family: var(--vp-font-family-base);
   z-index: var(--vp-z-index-local-nav);
   background-color: var(--vp-code-copy-code-bg);
-  color: var(--vp-code-copy-code-active-text);
-  color: white;
+  /* use theme variable so text adapts to light/dark modes */
+  color: var(--vp-code-copy-code-active-text);
   border: 1px solid var(--vp-code-copy-code-border-color);
   padding: 5px 10px;
   border-radius: 5px;
   transition: all 0.3s ease;
 }

2 changes: 2 additions & 0 deletions packages/altair-docs/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"@types/lodash.sortby": "^4.7.9",
"altair-graphql-core": "workspace:*",
"glob": "^11.0.0",
"hast-util-to-string": "^3.0.1",
"inflection": "^3.0.0",
"lodash.escaperegexp": "^4.1.2",
"lodash.sortby": "^4.7.0",
Expand All @@ -23,6 +24,7 @@
"markdown-it-title": "^4.0.0",
"node-cache": "^5.1.2",
"parent-module": "^3.1.0",
"shikiji": "^0.10.2",
"transliteration": "^2.3.5",
"vitepress": "^1.0.0-rc.39",
"vitepress-plugin-google-analytics": "^1.0.2",
Expand Down
35 changes: 31 additions & 4 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading