-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Do not clear headers when switching between tabs #3552
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 8d499cb The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
good find, thank you! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3552 +/- ##
==========================================
- Coverage 60.80% 55.32% -5.48%
==========================================
Files 120 115 -5
Lines 5616 5348 -268
Branches 1487 1450 -37
==========================================
- Hits 3415 2959 -456
- Misses 1749 1962 +213
+ Partials 452 427 -25
|
@acao Would it be OK to merge? |
241bb5b
to
f87c895
Compare
I pushed another commit since it would just use whatever you have in the editor and not overwrite it, using default headers is a better choice when switching tabs. Before:
|
I have two PRs open that initially seem well received, how come we don't move forward and merge if they are perceived as good changes? @acao |
d7f9b27
to
94b09ad
Compare
@klippx sorry I have been super busy lately and there was something else I needed to check, I cannot remember what held me up last time but I think this should be good to go, perhaps @thomasheyenbrock if you have a chance to look and think this is good to go feel free to merge |
94b09ad
to
f75dcfe
Compare
ed4e1d6
to
f75dcfe
Compare
Sorry, I pushed a commit that was rebased on main, but removed it again since it contained weird stuff, will stop interfering unless you ask me to do something! 🙏 |
f75dcfe
to
8d499cb
Compare
@thomasheyenbrock can you take a look at this as well? 🙏 |
Is this broken in main?
Seems like a bad file pointer is checked in:
|
Issue
When switching between tabs, the default header state is lost and replaced with empty string.
Demo of the bug:
Screen.Recording.2024-03-06.at.10.47.10.mov
Codepen to verify/reproduce the issue:
In this example we are using defaultHeaders, and when using multiple tabs, upon reload the headers are cleared when switching tabs.
https://codesandbox.io/p/sandbox/determined-brook-rp683x?file=/package.json:11,23
Note: If you edit the headers before switching tabs, they are not cleared. I am guessing that they are then instead stored in the internal state.