-
Notifications
You must be signed in to change notification settings - Fork 108
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(laboratory): preflight editor components #6498
Open
jasonkuhrt
wants to merge
23
commits into
main
Choose a base branch
from
refactor/laboratory/preflight-modules
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.
+980
−869
Open
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
e3a3a07
improve(laboratory): validate editor content with TypeScript
jasonkuhrt 6fdd227
group laboratory e2e tests
jasonkuhrt de26096
Merge branch 'main' into console-1003
jasonkuhrt 2d772e4
doc rationale
jasonkuhrt ce4c80f
lint
jasonkuhrt 7dfbe35
pin
jasonkuhrt 3bcfb5d
fix
jasonkuhrt 78d4a87
fix type cast any
jasonkuhrt 2b725b2
feedback
jasonkuhrt 09077d0
lock
jasonkuhrt ebe9be3
mention hack
jasonkuhrt 7692fc4
wip
jasonkuhrt a526c7b
Merge branch 'main' into console-1003
jasonkuhrt bae2af3
just refactor
jasonkuhrt b69f4f2
tidy
jasonkuhrt aa8242e
hooks directory
jasonkuhrt 9e434ee
modal value composite
jasonkuhrt ea4dbcb
lint
jasonkuhrt 657d475
fix
jasonkuhrt d7559ea
no enum
jasonkuhrt 3993748
fix
jasonkuhrt 8a747bf
fix tests
jasonkuhrt 65b6fe8
trigger invisible change
jasonkuhrt 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
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| import { setMonacoEditorContents } from '../../support/monaco'; | ||
|
|
||
| export namespace cyLaboratory { | ||
| /** | ||
| * Updates the value of the graphiql editor | ||
| */ | ||
| export function updateEditorValue(value: string) { | ||
| cy.get('.graphiql-query-editor .cm-s-graphiql').then($editor => { | ||
| const editor = ($editor[0] as any).CodeMirror; // Access the CodeMirror instance | ||
| editor.setValue(value); | ||
| }); | ||
| } | ||
jasonkuhrt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * Returns the value of the graphiql editor as Chainable<string> | ||
| */ | ||
| export function getEditorValue() { | ||
| return cy.get('.graphiql-query-editor .cm-s-graphiql').then<string>($editor => { | ||
| const editor = ($editor[0] as any).CodeMirror; // Access the CodeMirror instance | ||
| return editor.getValue(); | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Opens a new tab | ||
| */ | ||
| export function openNewTab() { | ||
| cy.get('button[aria-label="New tab"]').click(); | ||
| // tab's title should be "untitled" as it's a default name | ||
| cy.contains('button[aria-controls="graphiql-session"]', 'untitled').should('exist'); | ||
| } | ||
|
|
||
| /** | ||
| * Asserts that the tab with the given name is active | ||
| */ | ||
| export function assertActiveTab(name: string) { | ||
| cy.contains('li.graphiql-tab-active > button[aria-controls="graphiql-session"]', name).should( | ||
| 'exist', | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Closes the active tab | ||
| */ | ||
| export function closeActiveTab() { | ||
| cy.get('li.graphiql-tab-active > button.graphiql-tab-close').click(); | ||
| } | ||
|
|
||
| /** | ||
| * Closes all tabs until one is left | ||
| */ | ||
| export function closeTabsUntilOneLeft() { | ||
| cy.get('li.graphiql-tab').then($tabs => { | ||
| if ($tabs.length > 1) { | ||
| closeActiveTab(); | ||
| // Recurse until there's only one tab left | ||
| return closeTabsUntilOneLeft(); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| export namespace preflight { | ||
| export const selectors = { | ||
| buttonGraphiQLPreflight: '[aria-label*="Preflight Script"]', | ||
| buttonModalCy: 'preflight-modal-button', | ||
| buttonToggleCy: 'toggle-preflight', | ||
| buttonHeaders: '[data-name="headers"]', | ||
| headersEditor: { | ||
| textArea: '.graphiql-editor-tool .graphiql-editor:last-child textarea', | ||
| }, | ||
| graphiql: { | ||
| buttonExecute: '.graphiql-execute-button', | ||
| }, | ||
|
|
||
| modal: { | ||
| buttonSubmitCy: 'preflight-modal-submit', | ||
| editorCy: 'preflight-editor', | ||
| variablesEditorCy: 'env-editor', | ||
| }, | ||
| }; | ||
| /** | ||
| * Sets the content of the preflight editor | ||
| */ | ||
| export const setEditorContent = (value: string) => { | ||
| setMonacoEditorContents(selectors.modal.editorCy, value); | ||
| }; | ||
|
|
||
| /** | ||
| * Sets the content of the variables editor | ||
| */ | ||
| export const setEnvironmentEditorContent = (value: string) => { | ||
| setMonacoEditorContents(selectors.modal.variablesEditorCy, value); | ||
| }; | ||
| } | ||
| } | ||
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
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.
Can we name this file
cypress.ts? I don't understand why we need to use an abbreviation, let alone why we need to prefix it with a underscore.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.
Oh happy to explain:
_achieves. It could also be nested directory or$prefix, whereascypress.tswould not achieve this. Can you propose something that would? Alternatively clarify that you don't care about about my goal here.cynamespace and what this module does is provide a kind ofcydedicated for laboratory. If that is not logical to you that's fine, but I think its not as random as your reaction to my change might suggest.Please let me know how the above changes, if at all, your feedback.
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.
In #6476 I have tried to appease the feedback with a name of
__cypress__.tswhich utilizes a file name pattern that is used elsewhere.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.
The
.cy.tsextension does this though right? If this is a utility library for testing rather than a test file, it can have a different extension which is excluded from the test file matching.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.
@jdolle I think we are talking about different things. At issue in my mind was keeping the utility file in the directory tree in a prefix or suffix position in the directory. In other words it is purely about the visual parsing in the file explorer. In regards to test file execution, as you said, that works fine. Does that clear up for you what I was trying to do?
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.
yeah.
I'm not very concerned with this visual distinction. Unless the directory gets very large, it's still easy enough to distinguish
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.
I think I should mention that I struggle parsing visuals and navigating inconsistency more than the average person. It could be that our thresholds significantly differ on some things.
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.
If co-locating at the edges of the directory are hurting others' usability of the project, which is the sense I'm getting but not sure, then I will try to find another way, perhaps just back into the catch all directory.