User tests: Successful: Unsuccessful:
Add Javascript code to change the other input slider accordingly
document.getElementById('jform_resize_w').value = parseInt(width, 10);
document.getElementById('jform_resize_h').value = parseInt(height, 10);
Dashboard
> Content
> Media
> Try to edit an image using pencil icon
> Go to Resize tab
npm i
Don't know
Status | New | ⇒ | Pending |
Category | ⇒ | JavaScript Repository NPM Change |
Labels |
Added:
NPM Resource Changed
?
|
I have tested this item
@rjharishabh I have questions regarding this PR.
Maybe question 1 and 2 were already discussed somewhere else, and I'm not aware of it. If you have any info about that, let us know.
P.S.: If always both sliders mode, keeping the image ratio, and we can't enter one of the values directly without the other one being changed, too, so there is no way to resize without keeping the ratio, then it doesn't make sense to have 2 sliders. One slider for scaling 0 to 100 % would be enough in that case.
@drmenzelit Can you check my above comments and the PR with respect to if it's desired functionality or not?
When we change the width (or height), then the other value will change but the range slider was not updated
So I created this PR to change the range slider accordingly
When we change the width (or height), then the other value will change but the range slider was not updated
Ah, silly me, have not watched the value, only the slider, when watching the "before PR" video.
By directly entering the width or height value
It works
This PR just updates the range slider according to the corresponding width and height.
By directly entering the width or height value
It works
This PR just updates the range slider according to the corresponding width and height.
@rjharishabh Yes, I've noticed meanwhile. Only remaining question is if it is accessible or not, see Brian's question above, but if not, then it is also not accessible without this PR, I think, so that might be an issue to be fixed separately.
@brianteeman In case if the change of the number field of the other dimension already is announced by a screenreader without this PR, would it need to announce the same change on the other slider in addition? Or would that be redundant?
If redundant and the announcement of the number field is missing without this PR, then the PR is good as it is and we should open a new, separate issue for the missing announcement.
If not redundant, it should be added with this PR for the slider, if missing.
@brianteeman As much as I see, the PR is an improvment for sighted users - very nice - but changes nothing concerning accessibility.
Status | Pending | ⇒ | Ready to Commit |
RTC
I'm in the studio filming today so couldn't check myself
I have tested this item
Now that I am in front of a computer and able to test - this is not correct.
The slider is an input tool not an indicator. It is correct that the slider for height doesnt change when the slider for width is used.
It is the output of the field that changes not the slider. So everything is working correctly without this pr
Status | Ready to Commit | ⇒ | Pending |
Status | Pending | ⇒ | Needs Review |
The slider is an input tool not an indicator.
Yes,
But here both the width field and the corresponding slider works as input and both should correspond to the same value because both for width( or height) but using different input types
Both slider and field have value and it should be same because the fields are same
@brianteeman I see this as n improvement. Values are the same with or without PR. Only the slider position is visible corresponding to the other value.
agree to disagree then. the slider is not an indicator of anything and shouldnt be used as one.
agree to disagree then. the slider is not an indicator of anything and shouldnt be used as one.
@brianteeman Does that justify an unsuccessful test result?
imho yes. this is wrong. can you find an example somewhere of anything else doing this
Hi,
General a11y issue:
While two values will be changed, the big question is where the focus is at which moment. I think this issue is not easy to solve, because assestive technologie can't really deal with this kind of technique. A potential possibility could be a script that give the value of both fields to a live region, which gets the focus when one value is changed. After that assestive technologie needs a skiplink to jump back. But what is the right field? Width or height? I thing to make this depenencies accessible is not so easy.
This pull request does't effect that issue, but is helpful for the Usability, because it leads to a consistent behavior, I think.
Consistent with what?
I havent been able to find another example of this type of behaviour anywhere (except where the position on the range slide indicates a value which it doesn't here)
@angieradtke you are right, this is an issue and a challenge, and I have no solution. My opinion is, as I said - the PR is an improvment which changes nothing for accessibility.
@brianteeman - for me it is consistent to see the slider on the place where it matches the number in the field.
@brianteeman I can tell you an example where the slider is not used for doing the input but is adjusted to the input in another field: It's the hue setting in Atum's template style settings. Ok, there it is only one number value and one slider. But it is what you complain about. The slider is a pure input, but when you enter a valid number in the number field and use the enter key, the slider moves to that value. I think you have seen that many times recently when testing the admin UI repaint. Here with 2 sliders it is nothing else: The number field of the other dimension is updated, and so should be the corresponding slider.
If the position of the slider matched the value (like it does with the hue in atum) then I would agree 10000%
In this case it does not. It is just used as a means of changing the number and nothing else.
again I say - show me an example of where the behaviour this PR creates is used. Not just joomla but anywhere
Status | Needs Review | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-05-09 11:44:03 |
Closed_By | ⇒ | rdeutz |
so sad that this has been merged - its completely wrong and no one has shown any example anywhere in anything where this incorrect behaviour is used.
Maybe because it's an improvement which makes it work like people expect it to work, and feedback here from all others except of Brian was positive?
Just my guess.
Dear Brian , @brianteeman
I haven't quite understood your point of criticism yet.
These sliders are not suitable as a precise value transfer, because fine tuning with them is difficult. Nevertheless they are practical because you can quickly switch between value extremes ( very high/ very low) and in this case you can see the changes visually right away. Switch and input field in combination are very useful for sighted users in this case and complement each other. Therefore, the mutual value transfer makes sense here.
In terms of accessibility, only one input field would be better. However, this would affect the usability for sighted people.
OK let me try again.
In Joomla 4 we are using sliders as an input method in two places.
The first is for setting the hue of the admin template
Here the slider does TWO things. It's an indicator of value and an input tool
The second use is for editing images
Here the slider does ONE thing. Its an inaccessible input tool
If changing the position of the height slider had a purpose other than being a handle then I can see why you would want to move the width slider if it had a purpose. such as displaying the value
But in this case moving the width slider when you move the height slider serves no purpose as its not an indicator of anything. As I have repeatedly asked and no one has responded. Please show me an example anywhere of this behaviour.
All of the above is completely avoiding saying
Final demo showing that the position of the slider handle has no visual purpose
@brianteeman With your more detailed explanation I understand now. Sorry if I'm a bit slow with that. We're discussing on Glip what we should do, just revert this PR, or remove the sliders completely because they are not accessible. What would you suggest to do?
Step 1
Revert this PR
Step 2
Change the slider inputs so that they work (can be used to increase as well as reduce the size) and make it accessible.
Personally I would kill the slider
Note 1. there is the reverse issue with the rotate plugin. If you press an angle button then the text in in the input updates. But if you change the text in the field the button doesn't update
Note 2. the crop plugin doesnt use sliders
To be absolutely clear. the range slider is being used completely wrong here. It is called a range slider because it has a minimum and a maximum value. But you dont want it to be limited with a maximum value or you cant enlarge the image. The only type of slider you could have is one that had a linear scale. But they suck. This is why you hardly ever see sliders for image resize. Thats why I kept asking to be shown an example
After sleeping over it - I agree. Let' remove the slider.
To be honest: I haven't found the example Brian asked me to look for.
And I can live without the sliders.
So gor for it.
you cant find what doesnt exist ;)
creating a pr
@brianteeman I remembered having seen sliders for image resizing a long time ago, maybe end of the 1990's on Windows 3.11 or so, in some program, but when I check all kinds of programs now where you can resize or scale images I don't find any sliders at all. The all have number fields for entering height and width, and a check box if aspect ratio to be kept or not. So maybe I should forget about the old times ;-)
or perhaps your memory is thinking of something else ;)
I have tested this item✅ successfully on ae90ba6
Confirmed bug and fix works!
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33568.