[4.0] [WIP] Joomla Installation: multiple corrections when installing languages and more#32601
[4.0] [WIP] Joomla Installation: multiple corrections when installing languages and more#32601infograf768 wants to merge 3 commits intojoomla:4.0-devfrom
Conversation
| <?php endif; ?> | ||
| <div class="alert flex-column mb-1" id="removeInstallationTab"> | ||
| <?php if ($this->development) : ?> | ||
| <span class="mb-1 font-weight-bold"><?php echo Text::sprintf('INSTL_SITE_DEVMODE_LABEL', 'installation'); ?></span> |
There was a problem hiding this comment.
I know this was present before BUT the class font-weight-bold should be avoided in this case. It is being used as a visual indication of importance but as it is just css it has no semantic meaning. Instead it should be either an appropriate heading or the semantic "strong"
There was a problem hiding this comment.
so, what precisely do you suggest? Modifying both strings to add <strong> in the values?
In any case this is epiphenomenic in this PR and I will wait for real full tests
| <?php endif; ?> | ||
| <div class="alert flex-column mb-1" id="removeInstallationTab"> | ||
| <?php if ($this->development) : ?> | ||
| <span class="mb-1 font-weight-bold"><?php echo Text::sprintf('INSTL_SITE_DEVMODE_LABEL', 'installation'); ?></span> |
There was a problem hiding this comment.
| <span class="mb-1 font-weight-bold"><?php echo Text::sprintf('INSTL_SITE_DEVMODE_LABEL', 'installation'); ?></span> | |
| <span class="mb-1"><strong><?php echo Text::sprintf('INSTL_SITE_DEVMODE_LABEL', 'installation'); ?></strong></span> |
Why would you want to change a language string to add html markup
|
@infograf768 please don't re introduce the remove folder button. We can do this step automatically. I'll post some code in a bit Try these: |
|
My understanding of what you want to do is, when version is I tested your files without installing extra languages and setting version.php to stable
|
|
I can’t work in rn but this or as is shouldn’t be merged as it messes up ux. You don’t need extra button for normal installations they are confusing just use the 2 existing ones |
|
Setting this to WIP as the part concerning is still correct. |
I am sure you understand that this j4 installation code has been failing on many points since the beginning as it is extremely complex for little gain vs j3. |
|
@infograf768 I haven't tested the change but look like there is a small bug in the propose code by @dgrammatiko. Could you please try to replace the code: document.querySelectorAll('.removeInstallationFolder').map(function(el) {with [].slice.call(document.querySelectorAll('.removeInstallationFolder')).forEach(function (el) {I think it will fix the error you mentioned. |
|
@joomdonation |
|
works without installing languages :) |
|
If version is stable, then It works, but if version is dev (even when not installing languages), the Remove installation folder button has no effect and I can't see any error in console. Maybe we need to add back the old code as a supplement to the modified one to take care of that aspect? I mean adding back this deleted part EDIT: |
|
It works if I do Are you OK with this? If yes I can correct tabs/dots and other cs and modify my PR or make a new one. |
|
@infograf768 You don't need the extra if/lse with the same code could be : [].slice.call(document.querySelectorAll('.removeInstallationFolder')).forEach(function (el) {
el.addEventListener('click', function (e) {
e.preventDefault();
Joomla.request({
method: "POST",
url: Joomla.installationBaseUrl + '?task=installation.removeFolder&format=json',
perform: true,
token: true,
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
onSuccess: function () {
const customInstallation = document.getElementById('customInstallation');
customInstallation.parentNode.removeChild(customInstallation);
const removeInstallationTab = document.getElementById('removeInstallationTab');
removeInstallationTab.parentNode.removeChild(removeInstallationTab);
// We skip removing data for dev setups on the redirect buttons, unless someone clicks on the extra red button
let confirm =(e.target.dataset.isDev) ? window.confirm(Joomla.Text._('INSTL_REMOVE_INST_FOLDER').replace('%s', 'installation')) : undefined;
if (!confirm) { window.location.href = e.target.dataset.redirectTo; }
},
onError: function (xhr) {
Joomla.renderMessages({ error: [xhr] }, '#system-message-container');
}
});
}
});
}); |
|
@infograf768 my bad remove the |
|
not there yet. |
|
Also, if I use the Remove folder button and then click on cancel, it deletes the installation and open admin login... |
Where is this cancel button?
<div class="alert flex-column mb-1" id="removeInstallationTab">
<?php if ($this->development) : ?>
<span class="mb-1 font-weight-bold"><?php echo Text::sprintf('INSTL_SITE_DEVMODE_LABEL', 'installation'); ?></span>
<button class="btn btn-danger mb-1 removeInstallationFolder" data-is-dev="false" data-redirect-to="<?php echo Uri::root(); ?>administrator"><?php echo Text::sprintf('INSTL_COMPLETE_REMOVE_FOLDER', 'installation'); ?></button>
<?php endif; ?>
</div>
<?php echo HTMLHelper::_('form.token'); ?>
<div class="form-group j-install-last-step d-grid gap-2">
<button class="btn btn-primary w-100 removeInstallationFolder" data-is-dev="<?php echo (bool) $this->development; ?>" data-redirect-to="<?php echo Uri::root(); ?>"><span class="icon-eye" aria-hidden="true"></span> <?php echo Text::_('INSTL_COMPLETE_SITE_BTN'); ?></button>
<button class="btn btn-primary w-100 removeInstallationFolder" data-is-dev="<?php echo (bool) $this->development; ?>" data-redirect-to="<?php echo Uri::root(); ?>administrator"><span class="icon-lock" aria-hidden="true"></span> <?php echo Text::_('INSTL_COMPLETE_ADMIN_BTN'); ?></button>
</div> |
|
The js alert to confirm deleting the folder |
|
Why do you get an alert in the first place? This seems wrong can you add |
|
where do you want me to add |
onError: function (xhr) {
console.log(xhr)
Joomla.renderMessages({ error: [xhr] }, '#system-message-container');
} |
|
@infograf768 who added this confirm thing? This is totally wrong Edit: I see, you did that, please remove it. You don't have to ask, the installation folder NEEDS to be removed on the normal installations and for devs there's an extra button saying |
You are joking I hope!! It was you, before and after your proposal above. and it was there before you posted and it was working before as you deleted the old code |
|
I just copy/pasted the thing you provided that was repeating 20 lines of code for no reason. I didn't really process your code!!! |
|
This is what we have now in 4.0-dev. No comment. it is obvioulsy NOT my code. joomla-cms/installation/template/js/remove.js Lines 33 to 59 in cd4b0e1 |
|
Correct, also it was you that introduced this crap: 7162ed4 Please remove it, you're adding friction whenever you ask users anything. Just do the right thing, by default! Also for dev env, you don't even need a button, just say |
|
What I added concerned the string itself and has no impact on your failing code. That's where is the main "crap" as you nicely say. |
|
Do we need an issue for the remaining things? As I remember we had only a comment in #32516 . |
|
@richard67 Just look at the description of this PR and you will see what I tried to correct., specially the part concerning the languages. |
|
@infograf768 Instead of closing the PR, maybe leave it as how it is and let us know the bugs which need to be fixed so that we can try to get it sorted? |
@infograf768 I never implemented the removal of the installation, it was done here: #26276 So I apologise for the crap as it wasn't really you! That said the whole concept I had for the installation was simplification, ask only what needs to be answered. The way the removal of the installation was implemented is totally against that idea and frankly unacceptable. You don;t need to ask obvious things... |
|
There is no major bug in the PR itself except the will to suddenly change the way to delete the installation folder directly using the "complete & open...' buttons when version is stable. |
|
I totally disagree with the statement "You don't need to ask obvious things..." |
A dev using the git version of Joomla needs to answer a question if they are sure that they want to delete the installation folder? Well, obviously, we see the world from different lenses |
|
We do NOT have only matured devs testing. Many testers use the patched ready to go install pack provided when a PR is ready. These people do not use git or ide. |
Even so, what are the expectation when clicking on the button |
|
@dgrammatiko Please remember that you are debating with someone who believes that.... Oh forget it. Not worth the aggro |
|
Relax there is a lot of stress because it is urgent to launch Joomla 4, it is years behind schedule and the worst thing is that the community is weakening because it already seems like a utopia to think that it will be released. |
|
@infograf768 I assume that @Quy removed the release blocker tag because this is closed. Probably best to create a new issue outlining the problems (again) and then tagging that as a release blocker. (far too many release blockers have simply been resolved by removing the tag 😭 ) |



Summary of Changes
Preventing installing languages after "installation" folder is deleted.
Display "Congratulations! Your Joomla site is ready." after installing and set languages after deleting installation folder.
Add to the string displayed next to the
Remove "installation folderwhen using the beta versionAdded conditional and new string to be used when J4 version will be stable.
Testing Instructions
Install a clean Joomla dev. Use NPM.
Click the "Install Additional Languages" button.
Select between French (fr-FR), German (de-DE), Persian (fa-IR).
Click Next.
Select a language
Click "Set Default Languages" button
Click again the "Install Additional Languages" button and Next.
Select a language
Click "Set Default Languages" button
Click on the
Remove "installation" folderPatch and redo a new installation with the same steps. Use npm
Actual result BEFORE applying this Pull Request
Error if trying to set again a default language after deleting the installation folder
after installing and set languages after deleting installation folder we get only "Congratulations"
the string displayed next to the
Remove "installation" folderwhen using the beta version just states the development statusExpected result AFTER applying this Pull Request
No more possibility of setting default language after deleting the "installation" folder.
"Congratulations! Your Joomla site is ready." is displayed
When in development mode, the string is now:
If we set version.php to stable, we will get
Documentation Changes Required
None
Notes
I guess we can safely delete that fieldset and relative js and lang strings.
I have not yet tested with RTL and I have not normalised yet the use of tabs instead of dots in the js as well as the correct alignment in the php.
This should be done in another PR
As I added longer texts next to the
Remove "installation" folder, the css may need some tweeks. I think this also could be done in another PR.