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

Selected text to quote #2762

Merged
merged 12 commits into from Mar 24, 2015
Merged

Conversation

MissAllSunday
Copy link
Contributor

As per this www.simplemachines.org/community/index.php?topic=533350.0

This PR offers pretty basic support for handling selected text to quote.

It uses a separate quote button, this button will appear on mouseup event if the user selected some text on that .inner div

It was build to be as unobtrusive as possible to not mess up with the existing quote system so everything is contained in a single mouseup event.

The quickbuttons sf-js-enabled sf-arrows li does not really like long text so the text I chose for this: "Quote selected text" looks weird.

I deliberately did not include the check on XMLHttpRequest

Needs tons of testing of course.

Signed-off-by: Suki <suki@missallsunday.com>
Signed-off-by: Suki <suki@missallsunday.com>
@XinYenFon
Copy link
Contributor

It looks nice on basics :) What i find

  • .quickbuttons li a.quote_selected_button missing from CSS (just another line for .quickbuttons li a.quote_button)
  • clicking outside of "inner div" does not dismiss the selected quote option and it quotes the selected text.

Signed-off-by: Suki <suki@missallsunday.com>
@MissAllSunday
Copy link
Contributor Author

The quote_selected_button is just for reference it doesn't add any style, the other thing is on purpose to avoid crashing the client with multiple calls to onmouseup event.

@margarett
Copy link
Contributor

Forgive my GH ignorance :P but to test this I can just download your "quote" branch and setup 2.1 with it?

@XinYenFon
Copy link
Contributor

Because its under quickbuttons, it adds icon.
screenshot-localhost 2015-03-06 21-00-51

@MissAllSunday
Copy link
Contributor Author

Since this is a small Pr you can just directly copy the added changes into your local branch, just be sure to delete them if you are planing on keep on using that branch.

antes, yes I know its there I deliberately put it there, yes thats the weird thing I was talking about I thought that the li or the ul class will be capable of handling the icon without me having to use custom css.

@XinYenFon
Copy link
Contributor

margarett, You can get patch (add .patch to url) and open Git Bash (alike) navigate to your repo (create different branch > checkout) then use "git apply 2762.patch"

suki, nope, not in quickbuttons :)

@live627
Copy link
Contributor

live627 commented Mar 8, 2015

This is easy to abuse. Try to quote the same post twice...

@MissAllSunday
Copy link
Contributor Author

As in clicking the button twice? I don't know what you mean, more details are needed.

@XinYenFon
Copy link
Contributor

@MissAllSunday when i click inside "inner div" now it dismisses the Quote Selected button but doesn't do it for outside of "inner div".

Yeap, try to click twice (or more) based on your click rate, it quotes more than twice.

@MissAllSunday
Copy link
Contributor Author

I already said thats totally intentional. The whole event is attached to the .inner class, anything that happens outside that class isn't taken into consideration.

I still don't understand what you mean by click rate, the on click event doesn't use on. it uses one() which indicates that there will only be one click. When the butotn is clieked it gets hided again, preventing it form being clicked again

Need more details, where does this happen? on which button? do you mean the normal quote button? I did not modify that button.

@XinYenFon
Copy link
Contributor

well, did you tried to click on quote selected button more than once?

Also, using only inner div to dismiss button is surely weird way to do it and I'm 100% sure people call it bug. But I'm not going back there.

@MissAllSunday
Copy link
Contributor Author

OK, add an onmuseup even to the whole document to see what happens... If you are sure this is a bug then propose a fix for it.

When I tried to do more than one click on the "Quote selected text" it gets hidden after the first click... DO NOTE that I said the "Quote selected text" and NOT the normal "Quote" button...

Again.... for the Nth time.,... need MORE details... which button are you actually clicking?

Make sure that the event attached to the "Quote selected text" has one() instead of on()

$(document).one('click', '#quoteSelected_' + oSelectedID + ' a', function(e){

…ehaviour...

Signed-off-by: Suki <suki@missallsunday.com>
@XinYenFon
Copy link
Contributor

Well, I'm not saying its a bug, you designed it that way but I'm sure others will agree with me, regular users see that as bug. But if you want to keep it that way that's ok as well.

Edit: yeap after your last commit that seems fixed :)

@margarett
Copy link
Contributor

OK, so I finally got to test this.
The thing with the "double click" seems fine as I can't reproduce it. So I select a text from the post area and the button appears. I try to click it very fast and only the first click has any effect.

The "dismiss" might actually be an issue, simply because it memorizes whatever you selected.
Reproduce: Have a reply like this:

reply 123

another line

and another

Then select just the word "reply". Click outside of the post area (so that the "reply" text is not selected anymore but the button is still shown -- the "Quote selected text" button!)
Now repeat this, randomly selecting text from the reply text and clicking OUT OF THE POST area (again, like this, the text is not selected anymore but the button doesn't disappear)
When you did this several times, finally click the "Quote selected text" button. It will introduce everything that you selected before. Eg:

[quote author=admin link=msg=3 date=1426036567]reply [/quote]

[quote author=admin link=msg=3 date=1426036567]another [/quote]

[quote author=admin link=msg=3 date=1426036567]another[/quote]

[quote author=admin link=msg=3 date=1426036567]line[/quote]

[quote author=admin link=msg=3 date=1426036567]another line[/quote]

[quote author=admin link=msg=3 date=1426036567]and [/quote]

[quote author=admin link=msg=3 date=1426036567]123[/quote]

This actually works like a nice multi-quote 👅 although I'm not sure if this is by design or not...

BTW: can we have this in the full reply page? 😃

@MissAllSunday
Copy link
Contributor Author

It wasn't designed like that, at least not intentionally, its just the nature of the code the one that allows this kind of behavior.

This can be further abused by selecting something completely out of the scope of the .inner div, for example the entire main menu so I need to find a way to only select the text of the .inner div we are working on.

@MissAllSunday
Copy link
Contributor Author

OK, after checking this further, the whole approach is wrong, the on click event shouldn't be inside the onmouseup one...

Gotta re-do this all over again.

Signed-off-by: Suki <suki@missallsunday.com>
Signed-off-by: Suki <suki@missallsunday.com>
Signed-off-by: Suki <suki@missallsunday.com>
Signed-off-by: Suki <suki@missallsunday.com>
Signed-off-by: Suki <suki@missallsunday.com>
Signed-off-by: Suki <suki@missallsunday.com>
@MissAllSunday
Copy link
Contributor Author

OK, made a few improvements:

  • Added it to the post page
  • I forgot JS is way more picky than PHP so !var is not the same as var == false. In this case we should use == and not ===
  • Moved the code to its own file.
  • Added a check to make sure the selected text really belongs to the div that fired the onmouseup event
  • Added the class to the css file.

The weird "multiquote" stuff still happens, I actually kinda like it but I do see some people getting all confuse about it.

The button still doesn't hide intil you click the div again, dunno how to solve this really, perhaps a setTimeout()?

@XinYenFon
Copy link
Contributor

Notice: Undefined variable: post in C:\wamp\www\SMF2.1\Themes\default\Display.template.php on line 571
" id="msg_14">

Signed-off-by: Suki <suki@missallsunday.com>
live627 added a commit that referenced this pull request Mar 24, 2015
@live627 live627 merged commit 75c0467 into SimpleMachines:release-2.1 Mar 24, 2015
@MissAllSunday MissAllSunday deleted the quote branch October 31, 2017 18:45
@mikekaganski
Copy link

mikekaganski commented Aug 23, 2022

Triple click (to select the whole line) on the last line of the message disables the "Quote selected text" button.

For test: open https://forumooo.ru/index.php?topic=9244.msg63373;topicseen#msg63373 ; try to triple-click at the message's last line "В notepad++ использую тот плагин, но в Индизайне тоже проверил, с тем же результатом.".

Unlike the case when you drag mouse to select, the triple click seems to get something past the correct area, and the button gets disabled.

Triple-clicking other lines works fine.

The site uses SMF 2.1.2.

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

Successfully merging this pull request may close these issues.

None yet

5 participants