Skip to content

fix: replace the TabItem values with general ones #1394

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

Merged
merged 1 commit into from
May 28, 2024

Conversation

Efim-Kapliy
Copy link
Contributor

It will be easier for the user if you replace the TabItem values with general ones, because if you select a tab in one of the code examples, TypeScript, then other examples change asynchronously (i.e. the code examples will remain in JavaScript, or vice versa).

Replace the TabItem values with general ones, because if you select a tab in one of the code examples, TypeScript, then other examples change asynchronously (i.e. the code examples will remain in JavaScript, or vice versa).
Copy link

netlify bot commented May 21, 2024

Deploy Preview for testing-library ready!

Name Link
🔨 Latest commit 7caa8ef
🔍 Latest deploy log https://app.netlify.com/sites/testing-library/deploys/664cb7b486f70e00095eb4e9
😎 Deploy Preview https://deploy-preview-1394--testing-library.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@MatanBobi
Copy link
Member

Thanks @Efim-Kapliy but I'm not sure I'm following. Currently if a user changes a tab, the rest change accordingly. What is this PR aiming to solve?

@Efim-Kapliy
Copy link
Contributor Author

Okay, now I'm going to try to explain with illustrative examples.

This is the option that is currently displayed asynchronously:
photo_2024-05-22_12-46-47

The expected, synchronous option:
2024-05-22_12-51-37

I think it will be more understandable.

@MatanBobi
Copy link
Member

MatanBobi commented May 22, 2024

Thanks, I was under the impression that these already work.
What's the reason it's not working? The values were the same before (e.g. jsx/tsx), your change refactors it to be js/ts.

Edit: I understand now. We have one item with values of js/ts and two with values of jsx/tsx. Since these don't really affect the code styling as it's done in the markdown, were good with merging this.
Do you want to align it across the entire docs?

@Efim-Kapliy
Copy link
Contributor Author

Yes, that's right.

Copy link
Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

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

This looks good to me (I had to check 3-4 times to see understand what changed though 😅)!
Thanks for the PR.

Do you want to align it across the entire docs?

I verified that this page is the only place where we use these tab items (with ts(x) and js(x)) ~ the other pages use the name of the TL (react, angular, ...)

@timdeschryver timdeschryver merged commit c6a349c into testing-library:main May 28, 2024
4 checks passed
@timdeschryver
Copy link
Member

@all-contributors please add @Efim-Kapliy for docs

Copy link
Contributor

@timdeschryver

I've put up a pull request to add @Efim-Kapliy! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants