Skip to content

[4.0] Installation: Adding a hint on top of Password field#30402

Merged
Quy merged 3 commits intojoomla:4.0-devfrom
infograf768:4.0_installation_password_hint
Aug 19, 2020
Merged

[4.0] Installation: Adding a hint on top of Password field#30402
Quy merged 3 commits intojoomla:4.0-devfrom
infograf768:4.0_installation_password_hint

Conversation

@infograf768
Copy link
Copy Markdown
Member

@infograf768 infograf768 commented Aug 17, 2020

Summary of Changes

As installation password must now be at least 12 characters, this adds a hint on top of the field.

Testing Instructions

Patch and install a clean Jooomla

Actual result BEFORE applying this Pull Request

No way to know one needs 12 characters minimum to install joomla

Expected result AFTER applying this Pull Request

Screen Shot 2020-08-18 at 12 45 44

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels Aug 17, 2020
@brianteeman
Copy link
Copy Markdown
Contributor

brianteeman commented Aug 17, 2020

Hints are generally considered to be bad for accessibility even though they wont fail automated tests see https://www.smashingmagazine.com/2018/06/placeholder-attribute/

Placing the text between the label and the input and associating it with the input using aria-described by on the input seems to be the recommendation.

But please check with the accessibility team

@infograf768
Copy link
Copy Markdown
Member Author

We have many occurences of hint in core. Should it not be solved at that level?

@brianteeman
Copy link
Copy Markdown
Contributor

Please ask the accessibility team

@infograf768
Copy link
Copy Markdown
Member Author

@zwiastunsw
Anything you can do?

@chmst
Copy link
Copy Markdown
Contributor

chmst commented Aug 17, 2020

Why not expand the label?

@infograf768
Copy link
Copy Markdown
Member Author

The code for hint in the password field (/layouts/joomla/form/field/password.php) is:
`strlen($hint) ? 'placeholder="' . htmlspecialchars($hint, ENT_COMPAT, 'UTF-8') . '"' : '',

In core this is what we have for the Search field

<input type="text" name="filter[search]" id="filter_search" value="" class="form-control" aria-describedby="filter[search]-desc" placeholder="Search" inputmode="search">

It is a text field layout where we have among attributes
!empty($description) ? 'aria-describedby="' . $name . '-desc"' : '',

If I add the same attribute in the password field layout + a description in the xml, i.e.

		<field
			name="admin_password"
			type="password"
			label="INSTL_ADMIN_PASSWORD_DESC"
			description="INSTL_ADMIN_PASSWORD_LENGTH"
			hint="INSTL_ADMIN_PASSWORD_LENGTH"
			id="admin_password"
			class="form-control" // to take off
			required="true"
			autocomplete="new-password"
			validate="password"
		/>

I will get

<input type="password" name="jform[admin_password]" id="jform_admin_password" value="" autocomplete="new-password" class="form-control form-control required" aria-describedby="jform[admin_password]-desc" maxlength="99" required="" data-min-length="4" placeholder="Enter at least 12 characters.">

Is it what you want for accessibility?

Note:
We also have to correct the double form-control class as it is already defined in the layout and is unecessary in the xml
!empty($class) ? 'class="form-control ' . $class . '"' : 'class="form-control"',

@brianteeman
Copy link
Copy Markdown
Contributor

you should remove the placeholder=
and add a div with an id of jform[admin_password]-desc that contains the text

@infograf768
Copy link
Copy Markdown
Member Author

So, what you mean is that the Search field is wrong?
There is a div for it indeed but as a tooltip role... It keeps the placeholder.

Screen Shot 2020-08-18 at 09 19 26

I think this is correct.

I can obtain a similar result for installation password with the change above and

<div class="password-group">
	<div class="input-group">
		<input
			type="password"
			name="<?php echo $name; ?>"
			id="<?php echo $id; ?>"
			value="<?php echo htmlspecialchars($value, ENT_COMPAT, 'UTF-8'); ?>"
			<?php echo implode(' ', $attributes); ?>>
		<?php if (!empty($description)) : ?>
			<div role="tooltip" id="<?php echo $name . '-desc'; ?>">
				<?php echo htmlspecialchars(Text::_($description), ENT_COMPAT, 'UTF-8'); ?>
			</div>
		<?php endif; ?>
		<span class="input-group-append">
			<button type="button" class="btn btn-secondary input-password-toggle">
				<span class="fas fa-eye fa-fw" aria-hidden="true"></span>
				<span class="sr-only"><?php echo Text::_('JSHOWPASSWORD'); ?></span>
			</button>
		</span>
	</div>
</div>

to get
Screen Shot 2020-08-18 at 10 01 35

Remains to add the css which is absent from installation

// set up hidden tooltip
[role="tooltip"]:not(.show) {
  z-index: $zindex-tooltip;
  display: none;
  max-width: 100%;
  padding: .5em;
  margin: .25em;
  color: $white;
  text-align: start;
  background: $black;
  border-radius: .2rem;
}

// reveal associated tooltip on focus
:focus + [role="tooltip"],
:hover + [role="tooltip"] {
  position: absolute;
  display: block;
}

Agree?

@brianteeman
Copy link
Copy Markdown
Contributor

reading on my phone but that would be correct if you wanted it as a "tooltip" style

@infograf768
Copy link
Copy Markdown
Member Author

I can get, after I take off the hint

Screen Shot 2020-08-18 at 10 58 58

and code is

Screen Shot 2020-08-18 at 11 00 00

Don't know any way to display a <div> in the field itself to replace the hint.

@brianteeman
Copy link
Copy Markdown
Contributor

Don't know any way to display a

in the field itself to replace the hint.

You definitely do not want to do that. That is what the article was explaining makes the hint not accessible.
You can either do it as you have done as a "tooltip" or as below which is what I thought you would be doing

image

@brianteeman
Copy link
Copy Markdown
Contributor

Or do as @chmst suggested
image

@infograf768
Copy link
Copy Markdown
Member Author

as below which is what I thought you would be doing

Easy enough. I prefer

Screen Shot 2020-08-18 at 12 40 40

or, with text-muted

Screen Shot 2020-08-18 at 12 45 44

@infograf768 infograf768 changed the title [4.0] Installation: Adding a hint in Password field [4.0] Installation: Adding a hint on top of Password field Aug 18, 2020
@infograf768
Copy link
Copy Markdown
Member Author

It can now be tested. Accessibility should be OK.
Screen Shot 2020-08-18 at 17 16 47

@Quy
Copy link
Copy Markdown
Contributor

Quy commented Aug 18, 2020

Please consider doing it this way:
30402

Add the following after line 114 and delete lines 95-99.

	<?php if (!empty($description)) : ?>
		<small id="<?php echo $name . '-desc'; ?>" class="form-text text-muted">
			<?php echo htmlspecialchars(Text::_($description), ENT_COMPAT, 'UTF-8'); ?>
		</small>
	<?php endif; ?>

@brianteeman
Copy link
Copy Markdown
Contributor

@Quy please read the article I linked to which explains why this is a bad idea. Telling me after the input what I should have put in the input is not correct

@Quy
Copy link
Copy Markdown
Contributor

Quy commented Aug 18, 2020

OK. Keep it as it but change the markup slightly.

30402before

@infograf768
Copy link
Copy Markdown
Member Author

@Quy will do

@infograf768
Copy link
Copy Markdown
Member Author

@Quy
<small> (which means font-size: .8rem;) would not work on Macintosh if not inside a <div>. The hint is displayed to the right of the field.

I just added the small class to get the result proposed. I.e.

<?php if (!empty($description)) : ?>
	<div id="<?php echo $name . '-desc'; ?>" class="text-muted small">
		<?php echo htmlspecialchars(Text::_($description), ENT_COMPAT, 'UTF-8'); ?>
	</div>
<?php endif; ?>

Screen Shot 2020-08-19 at 08 45 41

Folks, please test.

@Magnytu2
Copy link
Copy Markdown

I have tested this item ✅ successfully on bebeccb

Works fine for me.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30402.

@Quy
Copy link
Copy Markdown
Contributor

Quy commented Aug 19, 2020

I have tested this item ✅ successfully on bebeccb


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30402.

@Quy
Copy link
Copy Markdown
Contributor

Quy commented Aug 19, 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30402.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 19, 2020
@Quy Quy added this to the Joomla 4.0 milestone Aug 19, 2020
@Quy Quy merged commit 17097f1 into joomla:4.0-dev Aug 19, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 19, 2020
@infograf768 infograf768 deleted the 4.0_installation_password_hint branch August 19, 2020 18:45
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
)

* [4.0] Installation: Adding a hint in Password field

* fix accessibility

* Added small
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Language Change This is for Translators

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants