-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Chrome: Adding the Permalink popup #1042
Conversation
Editing permalinks is not always authorised, a permalink format containing the "slug" is required to allow editing the slug. AFAIK It's not possible to fetch the permalink format using the Rest API right now. Thoughts on this @nylen |
NICE! Thanks for working on this. Let's make a few visual improvements. I can help with this, and it can be in a separate PR. For example, let's pretend the title field is a block. So it gets the same 2px gray block boundary as a selected block is. That lets us also position the permalink popup in the exact same coordinates as the block toolbars are. That is, indended from the left edge of the selected border the block padding distance, and 2px pushed down from the top border so it's "sitting on the border": Can we change the text of the copy button to say "Copied!" when you click? Can we make the permalink clickable, in addition to being editable? |
@jasmussen Thanks for the review, I've tried to address the feedback as I could. |
Love this! Really impressive work. Teensy nitpick — can we add the same fade animation to the title border as the normal blocks have? Otherwise, visually SOLID. Thank! 👍 |
0c6da2f
to
6434533
Compare
return ( | ||
<Button | ||
ref={ ref => this.button = ref } | ||
className={ classes } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit confusing that that there's no click handler here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, that's how the clipboard
library works.
editor/post-title/index.js
Outdated
document.activeElement === textarea && | ||
textarea.selectionStart !== textarea.selectionEnd | ||
) { | ||
this.select(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might it be better to name this onSelect
?
} | ||
|
||
handleClickOutside() { | ||
this.setState( { isSelected: false } ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't blur
work here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't because of the popup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works nicely.
Created core ticket ( + patch ) for REST API |
Did this regress? I don't see it in master anymore... |
@jasmussen Did you save your post? |
Ah of course, that's it. |
} | ||
|
||
onSelectionChange() { | ||
const textarea = this.textareaContainer.textarea; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this doing? Can we add an inline comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #5422
refs #577
ClipboardButton
based on theclipboard
package