feat(pagination): remove number input arrow visibility

[jpuzz0]

Closes: #4224

To test, navigate to the Pagination examples and verify when focusing on any page select input box that up/down arrows are not visible and you are still able to use keyboard up/down arrows to increment/decrement the count.

[patternfly-build]

Preview: https://patternfly-pr-4641.surge.sh

A11y report: https://patternfly-pr-4641-a11y.surge.sh

[mcoker]

This looks really good! Left a couple of comments.

Also one more update needed - we calculate the width of that input with 2 variables:

• --pf-c-pagination__nav-page-select--c-form-control--width-chars - the number of characters the input should show.
• --pf-c-pagination__nav-page-select--c-form-control--width-base - the base width of the input including the estimated width of the arrows that were removed in this PR.

Now that the arrows are gone, we should update the calculation that creates space for them in the width of that input. Here's the block where those vars are defined:

patternfly/src/patternfly/components/Pagination/pagination.scss

Lines 40 to 42 in d57498c

	--pf-c-pagination__nav-page-select--c-form-control--width-base: 3.5ch;
	--pf-c-pagination__nav-page-select--c-form-control--width-chars: 2;
	--pf-c-pagination__nav-page-select--c-form-control--Width: calc(var(--pf-c-pagination__nav-page-select--c-form-control--width-base) + (var(--pf-c-pagination__nav-page-select--c-form-control--width-chars) * 1ch));

FWIW we also do this in the number input, which may be good as a reference:

patternfly/src/patternfly/components/NumberInput/number-input.scss

Lines 8 to 11 in d57498c

	// form control
	--pf-c-number-input--c-form-control--width-base: calc(var(--pf-global--spacer--sm) * 2); // element's padding
	--pf-c-number-input--c-form-control--width-chars: 4;
	--pf-c-number-input--c-form-control--Width: calc(var(--pf-c-number-input--c-form-control--width-base) + var(--pf-c-number-input--c-form-control--width-chars) * 1ch);

[mcoker]

Can you test adding these styles under the .pf-c-form-control selector below? That selector should target this element without the need to define input[...]

[mcoker]

This is great! We do this in number input using an older technique. Any interest in updating that as part of this PR, too? If not we can make that a separate issue:

patternfly/src/patternfly/components/NumberInput/number-input.scss

Lines 21 to 29 in d57498c

	// disable default number increment arrows
	// stylelint-disable
	-moz-appearance: textfield;
	&::-webkit-outer-spin-button,
	&::-webkit-inner-spin-button {
	-webkit-appearance: none;
	margin: 0;
	}
	// stylelint-enable

[mcoker]

This looks great, appreciate the DRY approach, and it matches the number input. Though I am noticing that there isn't always space for the number of characters to show via the --width-chars in both components. For example, "20" doesn't show correctly:

Seems like we should look into that a bit and make a small adjustment - do you agree @mcarrano @mmenestr?

Also this is going to change the visual width of the input, and if a user was either depending on the arrows and/or customized the input width, this could cause a visual issue for them. Just want to verify that's OK @mcarrano @mmenestr

[mcoker]

Awesome! I was going to suggest this but didn't want to overcomplicate things! Kudos for taking the initiative. 🔥

[mcarrano]

@mcoker Interestingly, I don't see the problem in Firefox, but I do in Chrome. I definitely think we need to address this, although I'm wary of adding more width since we could have cases where a pagination control is used in a toolbar with tight spacing. But is it true that overall we are reducing the with of the input by this PR? In that case maybe add some width back?

[jpuzz0]

I will say that I didn't recognize this in the react example equivalent for pagination because it seems we're automatically setting the number count to what the max is, so entering 20 out of 4 wasn't an option there, so it was definitely overlooked.

Thanks to the help of @mcoker who pointed out the input border was causing this issue, and for providing the calculation itself, I think these should be all set now:

[mcoker]

Awesome work @jeffpuzzo!! So much better!

[mcarrano]

Looks great @jeffpuzzo !

[patternfly-build]

🎉 This PR is included in version 4.172.0 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀