Skip to content
This repository has been archived by the owner on Jul 26, 2022. It is now read-only.

feat: ESLint Playground 🎉 #6

Merged
merged 48 commits into from
Apr 7, 2022
Merged

feat: ESLint Playground 🎉 #6

merged 48 commits into from
Apr 7, 2022

Conversation

snitin315
Copy link
Contributor

@snitin315 snitin315 commented Mar 6, 2022

Preview Link - https://deploy-preview-6--dreamy-nightingale-a71351.netlify.app/

Task List -

  • Add code editor and lint code with eslint
  • Fix code when clicking on fix
  • Share URL functionality
  • Let users select configuration
  • Download configuration
  • Show the squiggle under the problem text in the editor area.
  • Show Popups
  • Improve code editor theme for dark mode.
  • Disable fix button if the error is not fixable.

@netlify
Copy link

netlify bot commented Mar 6, 2022

Deploy Preview for dreamy-nightingale-a71351 ready!

Name Link
🔨 Latest commit 5d9cf91
🔍 Latest deploy log https://app.netlify.com/sites/dreamy-nightingale-a71351/deploys/62480f439e32d70008769673
😎 Deploy Preview https://deploy-preview-6--dreamy-nightingale-a71351.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 settings.

@eslint-github-bot
Copy link

Hi @snitin315!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

  • The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
  • There should be a space following the initial tag and colon, for example 'feat: Message'.
  • The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

@snitin315 snitin315 changed the title ESLint Playground 🎉 feat: ESLint Playground 🎉 Mar 6, 2022
@snitin315
Copy link
Contributor Author

Yeah. Something wrong with the last commit. I will fix it.

@snitin315
Copy link
Contributor Author

@SaraSoueidan can you check once again?

@snitin315
Copy link
Contributor Author

One thing I noticed: it’s easy to accidentally zoom the page on mobile. Can we disable that?

@nzakas Yes, disabled.

@SaraSoueidan
Copy link

Yesss I see it now. Oh Gosh this is exciting seeing it come to life like that! :D

Quick question: the select and multi-select dropdowns in the config area are not the accessible ones I provided. Is there a reason why you're using the ones you're using and not the HTML I provided?

@snitin315
Copy link
Contributor Author

Quick question: the select and multi-select dropdowns in the config area are not the accessible ones I provided. Is there a reason why you're using the ones you're using and not the HTML I provided?

@SaraSoueidan Yes, actually I had to migrate all the HTML components to react so that I can pass props and reuse them. The migration for select components seemed a bit complicated to me.
So, I went ahead with the react-select library for faster development. And we can actually override its styles however we like and I was able to get something close to our theme.

@snitin315
Copy link
Contributor Author

@nzakas @SaraSoueidan Is this as per design that the popup is dark in the light theme?

Screenshot 2022-03-30 at 7 25 28 AM

@SaraSoueidan
Copy link

@snitin315 The problem isn't with the styles, it's with the markup. The code isn't accessible and it's also not even properly operable by keyboard. Styles can be overridden but the markup is where the accessibility is built into. Changing the styles is not enough.

@SaraSoueidan
Copy link

@nzakas @SaraSoueidan Is this as per design that the popup is dark in the light theme?

Screenshot 2022-03-30 at 7 25 28 AM

Yes, this is per design. The popup is dark in light theme, and a lighter grey in dark theme.

@snitin315
Copy link
Contributor Author

@snitin315 The problem isn't with the styles, it's with the markup. The code isn't accessible and it's also not even properly operable by keyboard. Styles can be overridden but the markup is where the accessibility is built into. Changing the styles is not enough.

Oh, I see. I will work on migrating to the accesible select component you provided once everything else is done.

@SaraSoueidan
Copy link

@snitin315 The problem isn't with the styles, it's with the markup. The code isn't accessible and it's also not even properly operable by keyboard. Styles can be overridden but the markup is where the accessibility is built into. Changing the styles is not enough.

Oh, I see. I will work on migrating to the accesible select component you provided once everything else is done.

Awesome! Thank you ✨

@nzakas
Copy link
Member

nzakas commented Mar 31, 2022

I’d suggest migrating the components after this PR is merged. I want to make sure we can make incremental improvements rather than trying to do everything in this branch.

So can we say the goal for this branch is to make the playground functional, and then we can tweak the parts that need fixing afterwards?

@snitin315
Copy link
Contributor Author

Yeah, makes sense 👍

@snitin315
Copy link
Contributor Author

@nzakas I believe the playground is functional now.

One thing that is not done yet is showing multiple fixes in the tooltip/popup in case of suggestion. We do show them in the console section though.

Screenshot 2022-04-02 at 2 29 55 PM

Popup works fine in case of a single fix.

Screen.Recording.2022-04-02.at.2.32.54.PM.mov

I suppose we can work on this in a separate PR as an enhancement (I can open an issue to keep track). Few more things to be done separately -

  • Adding eslint and prettier
  • removing stale html/css/js files
  • Adding GitHub actions workflow
  • Bug: Color contrast #8

@snitin315 snitin315 marked this pull request as ready for review April 2, 2022 09:08
@snitin315
Copy link
Contributor Author

Marking this PR as ready for review, let me know if anyone finds any bugs.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

I think that this is a good stopping point for this PR. Basic functionality all seems to be working now. We can continue to iterate with smaller PRs.

Just leaving open in case anyone finds anything worth mentioning in the next couple days.

@nzakas nzakas merged commit 5f2dd18 into main Apr 7, 2022
@nzakas nzakas deleted the feat-playground branch April 7, 2022 00:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants