Skip to content
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

fix: Clicking in the body of a Markdown component does not put it into edit mode #32335

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

notHuman9504
Copy link

@notHuman9504 notHuman9504 commented Feb 20, 2025

I have created a on click event on div which is helping to fix issue.

fixes: #32252

@dosubot dosubot bot added the dashboard:editmode Related to te Dashboard edit mode label Feb 20, 2025
Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

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

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Fix Detected
Design Redundant Edit Mode State Logic ▹ view
Files scanned
File Path Reviewed
superset-frontend/src/dashboard/components/menu/WithPopoverMenu.tsx
superset-frontend/src/dashboard/components/gridComponents/Markdown.jsx
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx

Explore our documentation to understand the languages and file types we support and the files we ignore.

Need a new review? Comment /korbit-review on this PR and I'll review your latest changes.

Korbit Guide: Usage and Customization

Interacting with Korbit

  • You can manually ask Korbit to review your PR using the /korbit-review command in a comment at the root of your PR.
  • You can ask Korbit to generate a new PR description using the /korbit-generate-pr-description command in any comment on your PR.
  • Too many Korbit comments? I can resolve all my comment threads if you use the /korbit-resolve command in any comment on your PR.
  • Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
  • Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.

Customizing Korbit

  • Check out our docs on how you can make Korbit work best for you and your team.
  • Customize Korbit for your organization through the Korbit Console.

Feedback and Support

Comment on lines 227 to 240
let nextState = {
...this.state,
editorMode: mode,
isEditing: mode === 'edit',
};

if(mode == 'edit') {
nextState = {
...this.state,
editorMode: 'edit',
isEditing: true,
};
console.log('here');
}

This comment was marked as resolved.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

@sadpandajoe sadpandajoe changed the title Fix issue #32252: solved the issue for switching preview and edit mod… Fix: Clicking in the body of a Markdown component does not put it into edit mode Feb 20, 2025
@sadpandajoe sadpandajoe changed the title Fix: Clicking in the body of a Markdown component does not put it into edit mode fix: Clicking in the body of a Markdown component does not put it into edit mode Feb 20, 2025
@sfirke
Copy link
Member

sfirke commented Feb 21, 2025

Thanks for the PR! Looks like there's linting failures, please see the logs of the failed tests and work to get prettier etc. pre-commit checks passing locally.

@notHuman9504
Copy link
Author

image
This both are solution if 1st one is verified then move with first one and cancel 2nd one.otherwise let me work on linting issues for second one.

Copy link
Contributor

@rusackas Processing your ephemeral environment request here. Action: up.

Copy link
Contributor

@rusackas Ephemeral environment spinning up at http://34.209.3.204:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.

Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

Seems like it's working, and the code seems fine. Looks like there are some CI issues (lining) to tend to, but I'll restart the failing tests in case they're just flaky.

@rusackas
Copy link
Member

Oh, actually, the test failures seem relevant. Would you be willing to take a look at the event triggers there and see if you can get things to pass? You can run the single test from the superset-frontend directory via npm run test src/dashboard/components/gridComponents/Markdown.test.jsx

@notHuman9504
Copy link
Author

Sure I am looking into it.

@notHuman9504
Copy link
Author

For some reason i am not able to run test locally, can you check this request, otherwise I will find solution for running test locally.

@sfirke
Copy link
Member

sfirke commented Feb 22, 2025

Yep I just approved test runs. I have had trouble getting my local environment set up too so I get it!

@notHuman9504
Copy link
Author

@sfirke why this one test is failing i do not understand, or is there any docs for running test locally?

@pull-request-size pull-request-size bot added size/L and removed size/M labels Feb 23, 2025
@sfirke
Copy link
Member

sfirke commented Feb 23, 2025

@rusackas this is now passing CI, could you take a look when you get a chance?

@sfirke
Copy link
Member

sfirke commented Feb 23, 2025

Oh maybe I spoke too soon, it looks like the last commit removed the failing test. @notHuman9504 I'm reluctant to have this pass CI by removing the test unless we understand why the test failed and feel that it was invalid. It would be better to fix the test if needed.

@pull-request-size pull-request-size bot added size/M and removed size/L labels Feb 24, 2025
@notHuman9504
Copy link
Author

hey @sfirke now you can check I have done a all nighter to fix it.

@notHuman9504
Copy link
Author

notHuman9504 commented Feb 24, 2025

@sfirke hey some test in data models is failing, i didn't do any changes in database can you re run test or tell me what's wrong?

@notHuman9504
Copy link
Author

@rusackas can you re run the test?

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

Successfully merging this pull request may close these issues.

Clicking in the body of a Markdown component does not put it into edit mode.
3 participants