Skip to content

fix(form-field): remove leading space for asterisk #12727

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

Closed

Conversation

devversion
Copy link
Member

@devversion devversion commented Aug 18, 2018

  • In order to align with the 2018 Material Design guidelines, the leading space before the required asterisk should be removed.

Based on the specs and by looking at MDC, there should be no leading space. In addition to removing the space, MDC seems to still shift the asterisk by 1px (potentially to make it look better on higher resolutions; I can notice a little improvement in UX with 4k as well).

0px 1px
image image

P2 because it should be merged prioritized for V7 beta. (spec alignment)

Fixes #12721

@devversion devversion added the target: major This PR is targeted for the next major release label Aug 18, 2018
@devversion devversion requested a review from mmalerba as a code owner August 18, 2018 12:27
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 18, 2018
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but some unit tests are failing.

@devversion devversion added action: merge The PR is ready for merge by the caretaker P2 The issue is important to a large percentage of users, with a workaround labels Aug 18, 2018
@devversion devversion added target: patch This PR is targeted for the next patch release and removed target: major This PR is targeted for the next major release labels Oct 19, 2018
@ngbot
Copy link

ngbot bot commented Oct 24, 2018

Hi @devversion! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

@devversion devversion added pr: needs rebase and removed action: merge The PR is ready for merge by the caretaker labels Oct 25, 2018
* In order to align with the 2018 Material Design guidelines, the leading space before the required asterisk should be removed.

Fixes angular#12721
@devversion devversion force-pushed the fix/form-field-space-asterisk branch from 1b37a84 to 9d86560 Compare October 26, 2018 18:07
@devversion devversion added action: merge The PR is ready for merge by the caretaker and removed pr: needs rebase labels Oct 26, 2018
@devversion devversion force-pushed the fix/form-field-space-asterisk branch from 9d86560 to 17132a3 Compare October 26, 2018 18:15
@devversion devversion added blocked This issue is blocked by some external factor, such as a prerequisite PR and removed action: merge The PR is ready for merge by the caretaker labels Oct 26, 2018
@devversion
Copy link
Member Author

Currently blocked on #13911 which should fix the Bazel tests.

@mmalerba mmalerba added aaa and removed aaa labels Apr 25, 2019
@devversion
Copy link
Member Author

@jelbourn should we still do this? I assume it's not really a priority anymore and eventually will be fixed by the MDC prototype. Closing for now. let me know if I should re-open.

@devversion devversion closed this Jul 10, 2019
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blocked This issue is blocked by some external factor, such as a prerequisite PR cla: yes PR author has agreed to Google's Contributor License Agreement P2 The issue is important to a large percentage of users, with a workaround target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

For text-fields with required attribute, remove space before asterisks (required text indicator)
4 participants