Skip to content

Conversation

Incogdino
Copy link
Contributor

  • Aligned svg image of copy plugin.
  • Added tooltip to provide better responsiveness.

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

#2581

Overview of changes:
The current svg of the copy plugin is slightly misaligned and offcentered after clicking once
image

Chanages made:

  • Aligned svg of the plugins.
  • Add tooltip to each plugin to provide readers information of what the plugin does.

image
image
image

Anything you'd like to highlight/discuss:

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)

Add tooltip for copy and wrap plugins

  • Aligned svg image of copy plugin.
  • Added tooltip to provide better responsiveness.

Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

Reviewer checklist:

Indicate the SEMVER impact of the PR:

  • Major (when you make incompatible API changes)
  • Minor (when you add functionality in a backward compatible manner)
  • Patch (when you make backward compatible bug fixes)

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

  • To be included in the release note for any feature that is made obsolete/breaking

Give a brief explanation note about:

  • what was the old feature that was made obsolete
  • any replacement feature (if any), and
  • how the author should modify his website to migrate from the old feature to the replacement feature (if possible).

Incogdino and others added 2 commits March 20, 2025 22:42
- Aligned svg image of copy plugin.
- Added tooltip to provide better responsiveness.
@lhw-1 lhw-1 added the r.Patch Version resolver: increment by 0.0.1 label Mar 20, 2025
@lhw-1
Copy link
Contributor

lhw-1 commented Mar 20, 2025

The "Copied!" success message does not seem to work on the deployed site (Firefox & Chrome) on Windows 10, on my end @Incogdino

@lhw-1
Copy link
Contributor

lhw-1 commented Mar 20, 2025

vid.mp4

Previously the constants were specified as string literals instead of
template literals.
@Incogdino
Copy link
Contributor Author

The "Copied!" success message does not seem to work on the deployed site (Firefox & Chrome) on Windows 10, on my end @Incogdino

Thanks for noticing this! I have updated the code and should work now!

@lhw-1
Copy link
Contributor

lhw-1 commented Mar 21, 2025

Thanks for the changes @Incogdino, I like the new changes for the copy button! The tooltips are quite useful.

As an addition, for the wrap button, I wonder if we can indicate the wrap state with css (perhaps light -> dark color scheme for the button when in wrap text mode)? At the moment, when clicking on the wrap text button, there is no visual response on click. We can even have text changes in the tooltip according to whether it is in "wrap mode" or not, though that might be optional if the visual changes are handled well.

@Incogdino
Copy link
Contributor Author

As an addition, for the wrap button, I wonder if we can indicate the wrap state with css (perhaps light -> dark color scheme for the button when in wrap text mode)?

@lhw-1 That is a good suggestion. We can try a colour combination that will not look out of place. For the text changes within the tooltip of text wrap, I'm thinking perhaps a change between "Wrap text" and "Unwrap text" depending on whether the text is wrapped.

@lhw-1
Copy link
Contributor

lhw-1 commented Mar 27, 2025

We can try a colour combination that will not look out of place. For the text changes within the tooltip of text wrap, I'm thinking perhaps a change between "Wrap text" and "Unwrap text" depending on whether the text is wrapped.

Sounds good to me. Any updates on the PR? @Incogdino

1. Added visual changes to wrap button on activate and deactivate to
denote that the code is wrapped.
2. Updated wrap button tooltip on activate and deactivate.
@Incogdino
Copy link
Contributor Author

We can try a colour combination that will not look out of place. For the text changes within the tooltip of text wrap, I'm thinking perhaps a change between "Wrap text" and "Unwrap text" depending on whether the text is wrapped.

Sounds good to me. Any updates on the PR? @Incogdino

I have just added changes to the wrap button.

image
It now changes to a slightly greyish tone to simulate that the code is currently wrapped.

The tooltip text also changes to "unwrap text" to indicate that the next click would unwrap the code

Copy link
Contributor

@lhw-1 lhw-1 left a comment

Choose a reason for hiding this comment

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

I like the new dynamic changes - LGTM. Thanks @Incogdino!

@lhw-1 lhw-1 merged commit e3b6dd4 into MarkBind:master Mar 28, 2025
8 checks passed
Copy link

@lhw-1 Each PR must have a SEMVER impact label, please remember to label the PR properly.

@gerteck
Copy link
Contributor

gerteck commented Apr 3, 2025

Love this, this is awesome! The wrap text tooltip is very useful imo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
r.Patch Version resolver: increment by 0.0.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants