fix: use the default query on adding a new tab

[cimdalli]

When a new tab is added, it sets the query value as an empty string instead of defaultQuery value. Tab default query value should be defaultQuery variable if it's set on the component. Otherwise, it should be DEFAULT_QUERY constant.

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: a75583ed551499050576d821f46b8d64131c4541

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@graphiql/react	Patch
graphiql	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: cimdalli / name: Okan Cetin (544cbe137b0ed3ee58d453384f9652b510d605a3, 94b5645fcfdfaf0d311932e7f4ce887aca41fe48, 1f497d3cfc731357014f432adcb207df90709793, 64aa227dd7397072ba586b8db5397db43dde13aa, 00aba1d082821a328991d490be4205c348c8d95b)
• :white_check_mark: login: acao / name: Rikki Schulte (ef4b1f720be65333def62afd6d55bcf9979b3566)
• :white_check_mark: login: dimaMachina / name: Dimitri POSTOLOV (a75583ed551499050576d821f46b8d64131c4541, fcf470e711607552e3940d1368c0abd943a25bfe, be08c8fa3b81467922efe9fb16bf7270e2da4efe)

[acao]

makes sense to me. do any other @graphql/graphiql-maintainers have feelings about this?

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 67.29%. Comparing base (40bbd61) to head (a75583e). Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3441      +/-   ##
==========================================
- Coverage   67.30%   67.29%   -0.01%     
==========================================
  Files         121      121              
  Lines        7016     7017       +1     
  Branches     2263     2263              
==========================================
  Hits         4722     4722              
- Misses       2277     2278       +1     
  Partials       17       17              

Files	Coverage Δ
packages/graphiql-react/src/editor/context.tsx	76.42% <100.00%> (+0.19%)	:arrow_up:

... and 1 file with indirect coverage changes

[benjie]

I'm for this change 👍

[acao]

two things:

1. the cypress suite is failing on some tests you added
2. the headers prop is a bit nuanced - however if it is set, then defaultHeaders should always be overridden. query, variables and headers prop, and soon extensions, should drive a controlled component state, at least I think? For 4.x I'm considering dropping all the controlled component props with a migration path to the @graphiql/react hooks instead, because it leads to a much more perfomant and easier to implement controlled component implementation

[acao]

@cimdalli so it seems this clashes a bit with previously expected behavior:

when headers are supplied, they should only set the first tab, and not be used for new tabs apparently?

so we need to revert this for headers and do the same for query/defaultQuery. otherwise, it's a change in functionality and requires a new minor version

CC @dimaMachina who added tests for this expectation, what do you think?

[fieldsco]

Any update on this?

[klippx]

Is everyone happy with this change? I find it quite a bit annoying to have the "welcome message" shown every time you open a new tab... 😞

Aside from a UX point of view, I also suddenly must find a way to run my playwright test to clear the entire editor before typing in a query after clicking "+" new tab button.

I know this is too much to ask, but I would almost like the old behaviour back (or a way to escape this new behaviour)

[dimaMachina]

@klippx you can set defaultQuery="" in <GraphiQL /> maybe?

[klippx]

I decided to keep the default query as we assert on text from it to ensure the user is logged in and page is fully loaded and that the test can be started. defaultQuery='' is an option, but makes it harder to know that the page is loaded. To get things working in playwright we simply clear the editor using a + directive before filling it with input:

  const editorTextbox = page.getByLabel('Query Editor').getByRole('textbox')
  await editorTextbox.press('ControlOrMeta+a')
  await editorTextbox.press('ControlOrMeta+x')

It is more a matter of the UX: Each time a new tab is opened the defaultQuery is filled, which previously was only shown on the initial tab (something I have gotten used to). Imagine if your browser showed a tutorial screen every time you opened a new tab, or your favorite text editor showed a whole page of documentation every time you hit CMD+N.

When creating a new tab - you want a blank screen, at least that's my opinion. However it is fine if it is shown on inital tab on initial load. Perhaps an option defaultQueryBehaviour = 'initial' | 'always' is what I would consider, and maybe this extra amount of complexity would be merited.

[EDIT: this comment is not formatted well for some reason, I am not sure what is wrong with it]