Skip to content
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

Align label with checkbox #21660

Closed
wants to merge 1 commit into from
Closed

Align label with checkbox #21660

wants to merge 1 commit into from

Conversation

Gusted
Copy link
Contributor

@Gusted Gusted commented Nov 2, 2022

image

- Follow up from
go-gitea#21655 (comment)
- Use vertical-align and line-height to get it to align exactly with the checkbox.
@Gusted Gusted added this to the 1.19.0 milestone Nov 2, 2022
@Gusted Gusted added skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug labels Nov 2, 2022
@wxiaoguang
Copy link
Contributor

Take a look at other checkbox code. IIRC it should be like this:

<div class="ui checkbox">
    <label> the description </label>
    <input name="enable_update_checker" type="checkbox">
</div>

No more CSS style / ID / FOR is needed.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 2, 2022
@Gusted
Copy link
Contributor Author

Gusted commented Nov 2, 2022

image

That doesn't work, the current CSS always makes that the checkbox is left of the label(which for sake of consistency is not something we want) and there would be no label directly under the

` to add the correct amount of padding to align the field.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Nov 2, 2022

It should work. I made a fix. The problem was that the width of label was polluted before.

dd91b71

image

@Gusted
Copy link
Contributor Author

Gusted commented Nov 2, 2022

I'm not a big fan of moving the helper text inside ui checkbox and not marking it the help class anymore. Now you have text on the left and right side of the checkbox.

@wxiaoguang
Copy link
Contributor

I reverted my changes. But the left-label + vertical-align approach doesn't look good to me.

Maybe let other maintainers to decide.

@wxiaoguang wxiaoguang removed their assignment Nov 2, 2022
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 2, 2022
zeripath added a commit to zeripath/gitea that referenced this pull request Nov 2, 2022
The install page has been somewhat inconsistently styled for a while. This PR simplifies
and standardises the styling of these fields makes things line up better across widths.

Replace go-gitea#21660

Signed-off-by: Andrew Thornton <art27@cantab.net>
zeripath added a commit that referenced this pull request Nov 3, 2022
The install page has been somewhat inconsistently styled for a while.
This PR simplifies and standardises the styling of these fields makes
things line up better across widths.

Replace #21660

Signed-off-by: Andrew Thornton <art27@cantab.net>

Old:
![Screenshot from 2022-11-02
23-07-05](https://user-images.githubusercontent.com/1824502/199619007-4a6b66c5-e19c-4d29-b71b-9aa73f2789ca.png)


New:
![Screenshot from 2022-11-02
23-04-28](https://user-images.githubusercontent.com/1824502/199618779-370f88e7-b590-4abd-afb9-b66cc3194a5d.png)

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath
Copy link
Contributor

zeripath commented Nov 3, 2022

Replaced by #21668

@zeripath zeripath closed this Nov 3, 2022
@zeripath
Copy link
Contributor

zeripath commented Nov 3, 2022

Thanks @Gusted for spurring me to look at this

@Gusted Gusted deleted the align-label branch November 3, 2022 20:42
@lunny lunny removed this from the 1.19.0 milestone Nov 4, 2022
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants