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

Add a <kbd> button to the editor toolbar#5160

Closed
a2sheppy wants to merge 3 commits intomdn:masterfrom
a2sheppy:add-kbd-button
Closed

Add a <kbd> button to the editor toolbar#5160
a2sheppy wants to merge 3 commits intomdn:masterfrom
a2sheppy:add-kbd-button

Conversation

@a2sheppy
Copy link
Copy Markdown
Contributor

@a2sheppy a2sheppy commented Dec 7, 2018

This adds an option to the toolbar, adjacent to the <code> button,
which applies the <kbd> element to content. The <pre> button is
also moved to the right end of the toolbar button group it's in,
to place it closer to the syntax highlighting buttons.

This resolves these two bugs:

This adds an option to the toolbar, adjacent to the <code> button,
which applies the <kbd> element to content. The <pre> button is
also moved to the right end of the toolbar button group it's in,
to place it closer to the syntax highlighting buttons.

This resolves these two bugs:

* https://bugzilla.mozilla.org/show_bug.cgi?id=1209767
* https://bugzilla.mozilla.org/show_bug.cgi?id=1251506
@a2sheppy
Copy link
Copy Markdown
Contributor Author

a2sheppy commented Jan 4, 2019

I have no ability to set reviewers on this so consider this comment a request that one of @schalkneethling, @jwhitlock,or @Elchi3 give this a review. :)

@schalkneethling schalkneethling self-requested a review January 4, 2019 16:28
@schalkneethling
Copy link
Copy Markdown

After rebuilding the CKEditor source I can confirm that the editor toolbar is updated as expected:

screenshot 2019-01-07 at 15 04 39

@a2sheppy Just want to confirm that all relevant files are in the PR. The docs at https://kuma.readthedocs.io/en/latest/ckeditor.html#committing-changes state the following:

When updating CKEditor, adding a plugin, or changing the configuration, CKEditor should be rebuilt, and the results added to a new pull request.

Are there additional build files that need to be added to this PR, or is it not applicable? Thanks!

@a2sheppy
Copy link
Copy Markdown
Contributor Author

a2sheppy commented Jan 7, 2019

Updated with the built CKEditor.

@@ -0,0 +1,3 @@
{
"git.ignoreLimitWarning": true
} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@a2sheppy Was this intentionally added?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No... the instructions said that a git add on the plugins folder would add only the files that need to be, so I just did that and went with it. Didn't notice this going in there.

@schalkneethling
Copy link
Copy Markdown

Seems like a lot more files was added causing a whole list of conflicts. I wonder if all of these are required? https://kuma.readthedocs.io/en/latest/ckeditor.html#committing-changes

@jwhitlock Thoughts?

This file wasn't supposed to be committed
@a2sheppy
Copy link
Copy Markdown
Contributor Author

a2sheppy commented Jan 7, 2019

So, there's been another change to CKEditor since I initially submitted this PR, apparently. I will attempt to deal with it.

@a2sheppy
Copy link
Copy Markdown
Contributor Author

a2sheppy commented Jan 7, 2019

I have sadly exceeded my git skills at this point and am not sure how to proceed to fix this. I'm sorry about that. :(

@jwhitlock
Copy link
Copy Markdown
Contributor

I don't think it would be a good use of your time to try to repair this PR.

Here's the quick version:

  1. Start over with a branch from the current master on the Kuma repo
  2. Cherry-pick a941e9a onto that branch
  3. Re-build
  4. Submit a new PR

@jwhitlock jwhitlock closed this Jan 7, 2019
@a2sheppy
Copy link
Copy Markdown
Contributor Author

a2sheppy commented Jan 7, 2019

Will do. That's what I was thinking.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants