feat: tabs #2197

n1ru4l · 2022-02-22T12:39:08Z

No description provided.

changeset-bot · 2022-02-22T12:39:11Z

🦋 Changeset detected

Latest commit: 68a9382

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

This PR includes changesets to release 1 package

Name	Type
graphiql	Minor

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

codecov · 2022-02-22T13:27:16Z

Codecov Report

Merging #2197 (68a9382) into main (2d91916) will decrease coverage by 0.77%.
The diff coverage is 75.28%.

@@            Coverage Diff             @@
##             main    #2197      +/-   ##
==========================================
- Coverage   65.70%   64.93%   -0.78%     
==========================================
  Files          85       80       -5     
  Lines        5106     5338     +232     
  Branches     1631     1695      +64     
==========================================
+ Hits         3355     3466     +111     
- Misses       1747     1868     +121     
  Partials        4        4

Impacted Files	Coverage Δ
packages/codemirror-graphql/src/hint.ts	`94.73% <ø> (ø)`
packages/codemirror-graphql/src/lint.ts	`100.00% <ø> (ø)`
packages/codemirror-graphql/src/results/mode.ts	`47.05% <ø> (ø)`
...kages/codemirror-graphql/src/utils/forEachState.ts	`100.00% <ø> (ø)`
...ckages/codemirror-graphql/src/utils/mode-indent.ts	`0.00% <0.00%> (ø)`
packages/codemirror-graphql/src/variables/hint.ts	`89.70% <ø> (ø)`
packages/codemirror-graphql/src/variables/mode.ts	`79.48% <ø> (ø)`
packages/graphiql/src/utility/fillLeafs.ts	`5.33% <ø> (ø)`
...kages/graphiql/src/utility/introspectionQueries.ts	`100.00% <ø> (ø)`
packages/graphiql/src/utility/onHasCompletion.ts	`2.17% <0.00%> (ø)`
... and 68 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4686971...68a9382. Read the comment docs.

acao · 2022-02-22T15:14:31Z

@n1ru4l IIII seee what you're doing now... and it's freakin' fabulous :) 💯

packages/graphiql/src/components/GraphiQL.tsx

acao · 2022-02-23T08:47:43Z

this is looking awesome! i appreciate this immensely

part of the reason we wanted to rewrite the react componetry for this effort is GraphiQL.tsx itself

as this PR progresses, let's try to pull as much out of this monolith component, and into context providers & consumers as possible. otherwise, it will be very hard to expose useful APIs for user-defined components

acao · 2022-02-26T10:10:52Z

@n1ru4l today I will try to restore deploy previews so that the community can easily see your work and give feedback. thanks for this!

packages/graphiql/src/components/GraphiQL.tsx

acao

Let's see if we can architect this in a way that doesn't increase the complexity of the GraphiQL.tsx component as much

packages/graphiql/src/components/GraphiQL.tsx

acao · 2022-03-04T09:14:35Z

@n1ru4l are you up for a discord discussion perhaps? or we could even record a pairing session?

acao · 2022-03-04T09:45:48Z

awesome! maybe they can live in a seperate file? would these be useful to people building other IDEs, and thus maybe live in a state management abstraction such as @graphiql/toolkit? if folks are using immer or similar it might not, but for many this could be quite useful, whether using vuex, redux, zustand, etc. if this level of abstraction feels like over doing it, then no bother, but the more we can reduce complexity in this file and move to seperate files or even if it makes sense packages, the better. also recall we have a plan for moving react-specific components to @graphiql/react to be consumed by graphiql pkg, but we don't have to worry about that now either, just giving a near-term view of the refactoring plans.

acao · 2022-03-04T09:56:34Z

I still want to wait until deploy previews are working again so users can review changes like this. I have the day off because of my injury but I can wrap up that and a few other efforts hopefully

acao · 2022-03-08T09:52:46Z

@n1ru4l if you're able to make another commit, it should trigger a deploy preview now!

I will go through and update readme links later for the changed URLs, but this should work for now I think?

It disables some of the other deploy previews, and will be slow, but it at least gets us deploy previews for forked branches again

acao

This is ready to merge in my opinion. But I'd like it if someone else was able to review it!

acao · 2022-03-10T17:53:06Z

packages/graphiql/resources/renderExample.js

+  parameters.operationName = activeTab.operationName;
+  updateURL();
+}
+


Thanks for catching this detail! @harshithpabbati had a PR to add query parameter handling to the toolkit, which will make for a nice follow up. There is also the possibility of making it an internal detail in 2.0.0 if we are careful

packages/graphiql/src/components/GraphiQL.tsx

timsuchanek · 2022-03-11T22:18:46Z

This is ready to merge in my opinion. But I'd like it if someone else was able to review it!

I'll review this over the weekend!

timsuchanek · 2022-03-12T10:45:55Z

Quick update: I'm checking out the PR now, working on a few styling adjustments that I'll propose shortly.
One question for @n1ru4l: Did you try out, if subscriptions are correctly working when switching back and forth between tabs with running subscriptions?

acao · 2022-03-12T11:38:55Z

Though this doesn’t matter now with codemirror-graphql, a note for 2.0.0: with monaco-editor we will need to be sure to not destroy the editor instance when changing tabs and instead to editor.setModel() and change settings dynamically. This was a suggestion from the vscode team!

the model history is preserved in memory, so when you toggle back to a tab where you using before, the model undo/redo stack for that tab will still be available

timsuchanek · 2022-03-13T17:33:46Z

This is a great start, awesome! I just worked on some styling adjustments, here the before and after:

Before

before.mp4

After

after.mp4

Changes

If the tab title goes further than 100px, the tabs don't scale. In the playground, we just let the tabs grow with the title, that worked well so far, so that's also what I'm suggesting here
The "X" (close button) is now using the same "X" unicode as the docs explorer and the styling is strongly inspired by how VSCode shows the tabs
Instead of going for bold activated tabs, I suggest to just change the color for now, as the bold (especially for longer texts) makes the tabs jump around a littlebit
Taking the style of the toolbar buttons for the "+" was a good idea, however in the row of tabs it feels a bit out of place. I turned that into a more minimal design that should fit better into the tabs.
I adjusted the .active, :active and :hover background colors a bit - they seemed quite strong and I think the goal of showing which tab is active can be achieved with more smooth contrast

Let me know if you object any of the changes, happy to discuss them. For simplicity, I just went ahead and committed them already to this PR.
One thing I noticed, is that there's quite some delay between typing and the tab name updating. Not a big problem, just noticed that.
So once the subscriptions are tested and cleared, this can be merged from my side!

acao · 2022-03-14T07:36:38Z

@timsuchanek looks like you need to fix a few of the unit tests

timsuchanek · 2022-03-14T08:41:02Z

Thanks @acao, will do so in a bit!

packages/graphiql/src/components/Tabs.tsx

acao

We just need to use tab, tablist and tabpanel roles and other related aria attributes, for proper tab navigation

https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/tablist_role

this aria spec for tabs also determines which keyboard shortcuts we should use

update: switched to an approving review for now so as not to block, but with just a few attributes on a few components this could be made screen reader accessible

acao · 2022-03-22T08:50:09Z

@timsuchanek I think this last commit should fix the tests 🥳

timsuchanek · 2022-03-23T12:45:17Z

That's awesome! How do you want to go about the release? I'm happy to give it a try to learn the changeset workflow

acao · 2022-03-23T12:49:20Z

@timsuchanek would love for you to do the honors!

here are the steps I would follow:

verify the changeset we added here has everything we want to have in it - for the changelog, it will append to this file 'thanks @timsuchanek!' with a markdown link to your profile, to the commit and the PR
once you merge this PR, the Version Packages PR will be updated, and you can see the new changelog diff there
then, you can merge the Version Packages PR (see the note about CLA, it's fine despite the Linux CLA bot not liking the changeset bot)
then we should probably promote the new release on discord & twitter - would be great to automate this part eventually, but there is some nuance, since it's easy for us to fire off ~3-7 releases at once.

acao · 2022-03-23T16:13:52Z

@timsuchanek just rebased, good to go!

acao · 2022-03-23T18:00:55Z

@timsuchanek found another bug i introduced - tab add button is now a child of a button ! oops. chrome doesn't like this and throws a runtime console error

acao · 2022-03-26T12:34:46Z

@timsuchanek everything is looking good here! fixed that bug, found another regex issue with comments

timsuchanek · 2022-03-26T21:06:24Z

Awesome! Thanks for looking into it @acao! I'll look into it tomorrow ✌️

timsuchanek · 2022-03-27T07:53:24Z

This looks good, so I'm merging this now!

n1ru4l force-pushed the feat-tabs branch from 4f763ed to b6362ce Compare February 22, 2022 13:24

n1ru4l commented Feb 22, 2022

View reviewed changes

packages/graphiql/src/components/GraphiQL.tsx Show resolved Hide resolved

acao added the enhancement label Feb 23, 2022

n1ru4l force-pushed the feat-tabs branch from 066be6e to ab3f1bb Compare March 4, 2022 08:26

n1ru4l marked this pull request as ready for review March 4, 2022 08:29

acao reviewed Mar 4, 2022

View reviewed changes

packages/graphiql/src/components/GraphiQL.tsx Outdated Show resolved Hide resolved

acao requested changes Mar 4, 2022

View reviewed changes

packages/graphiql/src/components/GraphiQL.tsx Outdated Show resolved Hide resolved

acao self-requested a review March 4, 2022 09:55

acao approved these changes Mar 4, 2022

View reviewed changes

timsuchanek mentioned this pull request Mar 8, 2022

V2 monaco rewrite - Needs Maintainers & Contributors! graphql/graphql-playground#1366

Open

acao approved these changes Mar 10, 2022

View reviewed changes

timsuchanek self-requested a review March 11, 2022 13:55

acao reviewed Mar 14, 2022

View reviewed changes

packages/graphiql/src/components/Tabs.tsx Outdated Show resolved Hide resolved

acao requested changes Mar 14, 2022

View reviewed changes

acao approved these changes Mar 14, 2022

View reviewed changes

acao force-pushed the main branch from e0add6d to 3626f8d Compare March 23, 2022 16:01

n1ru4l and others added 16 commits March 23, 2022 17:09

feat: tabs

7d94490

extratc operation title for tabs

1179358

never persist response

69945da

feat: onTabChange event

b8cd9a2

test: simple Tab tests

3741a7d

adjust styles

6403ad5

make tab feature opt-in

6a790e6

refactor: smaller render function

d74f85d

refactor: move state updates to reducer-like functions

d29b1bd

fix: only persist headers if shouldPersistHeaders option is used

1a346c4

handle _ in op names, unit tests

da521f1

a11y tweaks, use button for tab, tabs prop optional

2e10b0f

adding changeset

4e31380

fix(tabs): styling adjustments 💅

abfd080

fix(tabs): fix styling

a6fb9bb

fix tab add tests

01531d4

acao force-pushed the feat-tabs branch from 0e2683b to 01531d4 Compare March 23, 2022 16:10

one more regex edge case, valid element nesting

68a9382

timsuchanek merged commit 3137a6c into graphql:main Mar 27, 2022

github-actions bot mentioned this pull request Mar 27, 2022

Version Packages #2251

Merged

n1ru4l deleted the feat-tabs branch July 5, 2022 11:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: tabs #2197

feat: tabs #2197

n1ru4l commented Feb 22, 2022

changeset-bot bot commented Feb 22, 2022 •

edited

Loading

codecov bot commented Feb 22, 2022 •

edited

Loading

acao commented Feb 22, 2022

acao commented Feb 23, 2022 •

edited

Loading

acao commented Feb 26, 2022

acao left a comment •

edited

Loading

acao commented Mar 4, 2022

acao commented Mar 4, 2022 •

edited

Loading

acao commented Mar 4, 2022

acao commented Mar 8, 2022 •

edited

Loading

acao left a comment •

edited

Loading

acao Mar 10, 2022 •

edited

Loading

timsuchanek commented Mar 11, 2022

timsuchanek commented Mar 12, 2022

acao commented Mar 12, 2022 •

edited

Loading

timsuchanek commented Mar 13, 2022 •

edited

Loading

acao commented Mar 14, 2022

timsuchanek commented Mar 14, 2022

acao left a comment •

edited

Loading

acao commented Mar 22, 2022

timsuchanek commented Mar 23, 2022

acao commented Mar 23, 2022 •

edited

Loading

acao commented Mar 23, 2022

acao commented Mar 23, 2022 •

edited

Loading

acao commented Mar 26, 2022 •

edited

Loading

timsuchanek commented Mar 26, 2022

timsuchanek commented Mar 27, 2022

feat: tabs #2197

feat: tabs #2197

Conversation

n1ru4l commented Feb 22, 2022

changeset-bot bot commented Feb 22, 2022 • edited Loading

🦋 Changeset detected

codecov bot commented Feb 22, 2022 • edited Loading

Codecov Report

acao commented Feb 22, 2022

acao commented Feb 23, 2022 • edited Loading

acao commented Feb 26, 2022

acao left a comment • edited Loading

Choose a reason for hiding this comment

acao commented Mar 4, 2022

acao commented Mar 4, 2022 • edited Loading

acao commented Mar 4, 2022

acao commented Mar 8, 2022 • edited Loading

acao left a comment • edited Loading

Choose a reason for hiding this comment

acao Mar 10, 2022 • edited Loading

Choose a reason for hiding this comment

timsuchanek commented Mar 11, 2022

timsuchanek commented Mar 12, 2022

acao commented Mar 12, 2022 • edited Loading

timsuchanek commented Mar 13, 2022 • edited Loading

Before

After

Changes

acao commented Mar 14, 2022

timsuchanek commented Mar 14, 2022

acao left a comment • edited Loading

Choose a reason for hiding this comment

acao commented Mar 22, 2022

timsuchanek commented Mar 23, 2022

acao commented Mar 23, 2022 • edited Loading

acao commented Mar 23, 2022

acao commented Mar 23, 2022 • edited Loading

acao commented Mar 26, 2022 • edited Loading

timsuchanek commented Mar 26, 2022

timsuchanek commented Mar 27, 2022

changeset-bot bot commented Feb 22, 2022 •

edited

Loading

codecov bot commented Feb 22, 2022 •

edited

Loading

acao commented Feb 23, 2022 •

edited

Loading

acao left a comment •

edited

Loading

acao commented Mar 4, 2022 •

edited

Loading

acao commented Mar 8, 2022 •

edited

Loading

acao left a comment •

edited

Loading

acao Mar 10, 2022 •

edited

Loading

acao commented Mar 12, 2022 •

edited

Loading

timsuchanek commented Mar 13, 2022 •

edited

Loading

acao left a comment •

edited

Loading

acao commented Mar 23, 2022 •

edited

Loading

acao commented Mar 23, 2022 •

edited

Loading

acao commented Mar 26, 2022 •

edited

Loading