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

Ensure commit-statuses box is sized correctly in headers #18538

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Feb 1, 2022

When viewing commits as commits the commit-status box will be fixed at 30px in height
due to being forced to be this size by a fomantic selector. This PR simply adds a
few more selectors to force this to have height auto.

Fix #18498

Signed-off-by: Andrew Thornton [email protected]

When viewing commits as commits the commit-status box will be fixed at 30px in height
due to being forced to be this size by a fomantic selector. This PR simply adds a
few more selectors to force this to have height auto.

Fix go-gitea#18498

Signed-off-by: Andrew Thornton <[email protected]>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 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 Feb 2, 2022
@zeripath
Copy link
Contributor Author

zeripath commented Feb 2, 2022

The requirement for this patch-up comes from _base.less:

gitea/web_src/less/_base.less

Lines 1932 to 1941 in 92e81e9

/* fix misaligned right buttons on box headers */
.ui.attached.header .right:not(.dropdown) {
position: absolute;
right: .78571429rem;
top: 0;
bottom: 0;
height: 30px;
margin-top: auto;
margin-bottom: auto;
}

Which appears to come from #13932

@silverwind
Copy link
Member

silverwind commented Feb 2, 2022

Can we remove the right class from the popup? That selector is meant to vertically center those right buttons on headers and it's obviously not ideal but I think a better fix is just to remove that class. The popup is absolutely positioned anyways, so I think right makes no sense on it.

@zeripath
Copy link
Contributor Author

zeripath commented Feb 2, 2022

Can we remove the right class from the popup? That selector is meant to vertically center those right buttons on headers and it's obviously not ideal but I think a better fix is just to remove that class. The popup is absolutely positioned anyways, so I think right makes no sense on it.

from repo-legacy.js:

// Commit statuses
$('.commit-statuses-trigger').each(function () {
const positionRight = $('.repository.file.list').length > 0 || $('.repository.diff').length > 0;
const popupPosition = positionRight ? 'right center' : 'left center';
$(this)
.popup({
on: 'click',
lastResort: popupPosition, // prevent error message "Popup does not fit within the boundaries of the viewport"
position: popupPosition,
});
});

The right/left says which side the popup appears.

@silverwind
Copy link
Member

silverwind commented Feb 2, 2022

Hmm I need to check further, I guess this is a bug in Fomantic because right inside a header (float right) has a different meaning than in a popup (anchor right).

@zeripath
Copy link
Contributor Author

zeripath commented Feb 2, 2022

it's our change in _base.less that has made this necessary. #18538 (comment)

@silverwind
Copy link
Member

silverwind commented Feb 2, 2022

Seems so, we should instead use a class name distinct from fomantic for these right-aligned header contents, which will then allow us to remove that troublesome rule.

@zeripath
Copy link
Contributor Author

zeripath commented Feb 2, 2022

Seems so, we should instead use a class name distinct from fomantic for these right-aligned header contents, which will then allow us to remove that troublesome rule.

Agreed but I think it's kinda difficult to even determine which things are relying on this selector - that's why I simply overrode the individual thing.

I wanted something that was simple to backport to 1.16 without having to make major changes across several templates files.

@zeripath
Copy link
Contributor Author

zeripath commented Feb 2, 2022

I think this might be a bit similar to our previous less code where we had at some point overridden right to always mean float right...


actually we still have this:

.ui {
  &.left:not(.action) {
    float: left;
  }

  &.right:not(.action) {
    float: right;
  }

AFAIU semantic had .float for these things so if you wanted something to float:left you'd set float left as the css and it would float left.

@silverwind
Copy link
Member

The .ui.attached.header .right:not(.dropdown) is specifically for vertically centering those right-aligned things in box headers which isn't something thats natively possible with fomantic. I will check the possiblilty of just adding a unique classname, it's a better solution for sure.

Ideally, I guess we should just flexbox all these headers instead, but it'll be a even more involved change.

@zeripath
Copy link
Contributor Author

zeripath commented Feb 3, 2022

Ideally, I guess we should just flexbox all these headers instead, but it'll be a even more involved change.

Why don't we do that after we've merged this and backported it to 1.16 then we can properly fix this.

Copy link
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

Let's go with this fow now, proper fix can follow.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 4, 2022
@lunny
Copy link
Member

lunny commented Feb 4, 2022

make L-G-T-M work

@lunny lunny merged commit 3c73741 into go-gitea:main Feb 4, 2022
@zeripath zeripath deleted the fix-18498-ensure-that-commit-statuses-in-header-resized branch February 4, 2022 15:34
zeripath added a commit to zeripath/gitea that referenced this pull request Feb 4, 2022
Somehow a spurious space sneaked in to go-gitea#18538
this PR simply removes it.

Signed-off-by: Andrew Thornton <[email protected]>
zeripath added a commit that referenced this pull request Feb 4, 2022
Somehow a spurious space sneaked in to #18538
this PR simply removes it.

Signed-off-by: Andrew Thornton <[email protected]>
zeripath added a commit to zeripath/gitea that referenced this pull request Feb 4, 2022
)

Backport go-gitea#18538
Backport go-gitea#18605

* Ensure commit-statuses box is sized correctly in headers

When viewing commits as commits the commit-status box will be fixed at 30px in height
due to being forced to be this size by a fomantic selector. This PR simply adds a
few more selectors to force this to have height auto.

Fix go-gitea#18498

Signed-off-by: Andrew Thornton <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>
zeripath added a commit to zeripath/gitea that referenced this pull request Feb 4, 2022
Somehow a spurious space sneaked in to go-gitea#18538
this PR simply removes it.

Signed-off-by: Andrew Thornton <[email protected]>
6543 pushed a commit that referenced this pull request Feb 4, 2022
…8606)

* Ensure commit-statuses box is sized correctly in headers (#18538)

Backport #18538
Backport #18605

* Ensure commit-statuses box is sized correctly in headers

When viewing commits as commits the commit-status box will be fixed at 30px in height
due to being forced to be this size by a fomantic selector. This PR simply adds a
few more selectors to force this to have height auto.

Fix #18498

Signed-off-by: Andrew Thornton <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>

* Remove the spurious space in the .ui.right additional selector

Somehow a spurious space sneaked in to #18538
this PR simply removes it.

Signed-off-by: Andrew Thornton <[email protected]>

Co-authored-by: wxiaoguang <[email protected]>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 5, 2022
* giteaofficial/main:
  Use `CryptoRandomBytes` instead of `CryptoRandomString` (go-gitea#18439)
  Remove the spurious space in the .ui.right additional selector (go-gitea#18605)
  Ensure commit-statuses box is sized correctly in headers (go-gitea#18538)
  [skip ci] Updated translations via Crowdin
  Prevent merge messages from being sorted to the top of email chains (go-gitea#18566)
  Fix pushing to 1-x-dev docker tag (go-gitea#18578)
  Replace `sync.Map` with normal maps (go-gitea#18584)
  Fix oauth docs usage for 2fa (go-gitea#18581)
  Update .gitattributes for .tmpl files (go-gitea#18576)
  Prevent panic on prohibited user login with oauth2 (go-gitea#18562)
  Fix manifest.tmpl (go-gitea#18573)
  Make docker gitea/gitea:v1.16-dev etc refer to the latest build on that branch (go-gitea#18551)
  Add dropdown icon to template loading dropdown (go-gitea#18564)
@zeripath zeripath added the backport/done All backports for this PR have been created label Feb 5, 2022
zeripath added a commit to zeripath/gitea that referenced this pull request Feb 7, 2022
Although go-gitea#18538 fixes the size of the header block the details links are still
coalescing.

Here we simply display: flex these instead of using .right

Signed-off-by: Andrew Thornton <[email protected]>
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
)

* Ensure commit-statuses box is sized correctly in headers

When viewing commits as commits the commit-status box will be fixed at 30px in height
due to being forced to be this size by a fomantic selector. This PR simply adds a
few more selectors to force this to have height auto.

Fix go-gitea#18498

Signed-off-by: Andrew Thornton <[email protected]>

* Update web_src/less/_repository.less

Co-authored-by: wxiaoguang <[email protected]>

Co-authored-by: wxiaoguang <[email protected]>
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
…tea#18605)

Somehow a spurious space sneaked in to go-gitea#18538
this PR simply removes it.

Signed-off-by: Andrew Thornton <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Commit status list squished on commit view
5 participants