I attach a patch which makes the admin interface easier to use.

Comments

justindodge’s picture

Version: 7.x-1.0-beta5 » 7.x-1.x-dev

Thanks for the patch. I think there are some good improvements in here.

I committed the patch to dev with a few adjustments:
1. I weighted the field "When external links are clicked" so that it sits above the 'warning text' textarea.
2. The "Warning page title" field is only applicable when the method is "warn using page", so I made it hidden when the other options are selected.
3. I adjusted the language used for the warning type options.
4. I did not think that "Open external links in a new window" should cause the timer option to be hidden - primarily because a user might never know that the timer option exists if they install extlink_extra with the 'new window' option checked. Further, hiding the timer field does not disable it's functionality if there is a value there, and so when I applied the patch my timer was activated but I could not see it because I had accidentally left "open links in new window" checked previously. I'm sure there is still an improvement to be made here, and I'm open to other suggestions/additional patches - but I'm going to leave it as is for the moment.
5. I am only hiding the 'warning text' field if the option "don't display a warning message" is checked - the confirm popup also supports text here.

justindodge’s picture

Status: Active » Fixed
tinycg’s picture

Hey Justin, as the person behind the usability improvements, I wanted to make sure I understood your concerns around item 4 above.

The module states that the timer won't work properly if links are opened in a new window. The reason I recommended hiding it was in an effort to not overwhelm the user with options that don't fit their configuration. With that said, you bring up a valid point.

Where I get a little lost is this, in the current implementation of the module whats the desired handling if a user does set the timer, AND sets to open links in a new window?

Maybe it would make sense to disable the field, and still store its value, so that when it is checked it wont interfere, is always visible, and when unchecked allows you to change it.

justindodge’s picture

@tinycg:

in the current implementation of the module whats the desired handling if a user does set the timer, AND sets to open links in a new window?

Well this one is starting to get a little tricky.
1. With no pattern matching going on, having both checked just means that the timer is being defeated - if you have a modal or page with the timer, it will just open the link in the same window after the countdown and ignore the 'new window' option. I recognize that this is a bit of a shortcoming on the UI.
Ideally, I would be able to just fix this. There are some hang ups with popup blocking behavior when javascript tries to trigger new windows, but I think the modal reaction can utilize an event capturing workaround. For an intermediate page, I don't think JS will ever be able to trigger a new window without user interaction.

Anyway failing a straight fix, your suggestion to just disable the 'new window' option by indicating that it's not in use/greyed out when the timer is on is a good one. Unfortunately, this is slightly complicated by pattern matching features.

2. There is a new feature now that allows you to suppress the click reaction of certain links but not others. ( #1451996: Open certain links in new window with no popup ) This gives us a new possibility to think about, lets say:
a. Open links in new window is ON
b. Timer is ON
c. Click reaction is MODAL
d. "Don't warn for links matching the pattern" is (pdf)

With this configuration you can get a functional benefit from having all of these options on - pdf's open directly in a new window, but normal links open in a modal with an automatic timer (same window).

--
I'm not really sure the best way to handle those scenarios in the interface, but I haven't given it a lot of thought either. For me, I don't know if it's worth trying to engineer anything too complicated to manage it, but I am fully welcome to more suggestions/patches.

justindodge’s picture

Version: 7.x-1.x-dev » 7.x-1.0-beta5

Version should be kept as 7.x-1.0-beta5 where the issue was noticed. Not sure why I changed that...

tamasd’s picture

Status: Fixed » Needs review
StatusFileSize
new3.35 KB

Further improvements:
- typo fixes
- language simplifications and fixes

temaruk’s picture

Small improvement upon latest patch, makes sure that the label of the text_format form field is not floated, as it appears misplaced in Firefox if it is floated.

justindodge’s picture

Status: Needs review » Fixed

Sorry for the delay here - some notes:

I could not apply the patches at #6 or #7. #7 I found had some absolute paths written in it, but even after removing them I think they may have become too out of date.

What I did do was go through #7 and manually apply portions of the patch that I liked. Some language I did not find to improve clarity, some was erroneous (for example, the warning text is used on the confirmation pop-up as well as the modal and page), but other portions I thought were helpful.

Thanks for your contributions! I think the easiest way to move on here is to see the latest code committed on dev and continue to revise (if so desired) and discuss individual issues.

I'm going to mark this issue fixed for now, but feel free to start additional issues as I know there are more improvements that could be made.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.