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

NEW Add option to open files in new tab #29604

Merged
merged 5 commits into from
May 5, 2024

Conversation

Vaadasch
Copy link
Contributor

@Vaadasch Vaadasch commented May 4, 2024

This is a feature that is recurringly requested along with the modification of "MAIN_DISABLE_FORCE_SAVEAS"

See for exemple :
https://www.dolibarr.org/forum/t/option-to-open-pdf-in-new-window/8389
https://www.dolibarr.org/forum/t/open-links-in-new-tab/14791
https://www.dolibarr.fr/forum/t/ouverture-des-pdf-dans-une-nouvelle-page/30766
https://www.dolibarr.fr/forum/t/pdf-dans-une-autre-fenetre/8058
https://www.dolibarr.fr/forum/t/ouverture-des-pdf/9447

In this PR, it is adding the option "MAIN_FILES_OPEN_NEWTAB" to open most files inside a newtab if it is not empty.

I will update the wiki once/if this PR is merge.

I suggest is to make this behavior as defaut, as it is the main behavior of all the main tools on the web.
In compensation, we could have an option "MAIN_FILES_OPEN_SAMETAB".
But it require consent from the experts who prefer otherwise, because the consensus will be status quo.

@Vaadasch
Copy link
Contributor Author

Vaadasch commented May 4, 2024

I've added the condition on MAIN_DISABLE_FORCE_SAVEAS so those who do not have it won't open a new tab when then have the popup to download the file.

I don't know how to solve the missing check

htdocs/core/class/html.formfile.class.php Outdated Show resolved Hide resolved
htdocs/core/class/html.formfile.class.php Outdated Show resolved Hide resolved
@Vaadasch
Copy link
Contributor Author

Vaadasch commented May 4, 2024

Thanks @hregis , i hadn't test enough 😞

@eldy
Copy link
Member

eldy commented May 4, 2024

I don't think we must introduce a second option. Because the feature is dependent on MAIN_DISABLE_FORCE_SAVEAS activation or not, i think it must be a third value of MAIN_DISABLE_FORCE_SAVEAS
MAIN_DISABLE_FORCE_SAVEAS=0 : Default (file is auto downloaded)
MAIN_DISABLE_FORCE_SAVEAS=1 : For to output the file into the browser tab
MAIN_DISABLE_FORCE_SAVEAS=2 : For to output the file into a browser tab open with target_blank

Because having MAIN_DISABLE_FORCE_SAVEAS off and MAIN_FILES_OPEN_SAMETAB on has no sense.
Don't you think so. if you agree, can you simplify the code to just test on MAIN_DISABLE_FORCE_SAVEAS=2 to add the target_blank ?

@eldy eldy added the PR to fix - See feedback in comments PR needs to be fixed to be integrated (some comments should describes the fix to do) label May 4, 2024
@Vaadasch
Copy link
Contributor Author

Vaadasch commented May 4, 2024

Of course, you're perfectly right.
It's done

@eldy eldy changed the title Add option to open files in new tab NEW Add option to open files in new tab May 5, 2024
@eldy eldy merged commit 7562eac into Dolibarr:develop May 5, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR to fix - See feedback in comments PR needs to be fixed to be integrated (some comments should describes the fix to do)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants