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

Fix fork button #6223

Merged
merged 10 commits into from Mar 6, 2019

Conversation

@jolheiser
Copy link
Member

jolheiser commented Mar 1, 2019

Fixes #6222

When disabling only the first part, I didn't like how semantic displayed it.
fork_basic

So instead I moved the basic class into an if-else.
It appears basic when a user can fork (no change from existing style)
It appears normal (but disabled) if they cannot fork. (image below)
fork_fix

There was also the case of a non-signed in user, which is why this is a separate if-else within the element to avoid any doubled up classes in favor of another logic flow.

@jolheiser jolheiser changed the title 6222 fix fork button Fix fork button Mar 1, 2019

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Mar 1, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@6460cff). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #6223   +/-   ##
=========================================
  Coverage          ?   38.83%           
=========================================
  Files             ?      355           
  Lines             ?    50256           
  Branches          ?        0           
=========================================
  Hits              ?    19518           
  Misses            ?    27909           
  Partials          ?     2829

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6460cff...673fa03. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 label Mar 1, 2019

@hexfs

hexfs approved these changes Mar 1, 2019

@GiteaBot GiteaBot added lgtm/need 1 and removed lgtm/need 2 labels Mar 1, 2019

@techknowlogick techknowlogick added this to the 1.8.0 milestone Mar 1, 2019

@silverwind

This comment has been minimized.

Copy link
Contributor

silverwind commented Mar 2, 2019

Still looks kind of weird, I'd fix the disabled CSS to use something like opacity: .5 on the whole button. Plus points for also adding cursor: not-allowed.

@silverwind

This comment has been minimized.

Copy link
Contributor

silverwind commented Mar 2, 2019

Also, I think some manual styling to make border-right change on hover would be nice.

@jolheiser

This comment has been minimized.

Copy link
Member Author

jolheiser commented Mar 2, 2019

After messing with CSS for a bit, this is what I came up with. Note that the right-button to get to forks is still available and the cursor is a pointer like normal.
It adds the cursor: not-allowed; change as well to the left-button. Since I needed to manually counteract any prior hover event styling, there's a chance I missed something that would show up differently in another browser.
Please let me know what further changes, if any, are needed.

fork-disabled

jolheiser added some commits Mar 2, 2019

New disabled look
Signed-off-by: jolheiser <john.olheiser@gmail.com>
@silverwind

This comment has been minimized.

Copy link
Contributor

silverwind commented Mar 2, 2019

I'd change the disabled class to something a bit more more generic like .disabled-repo-button so it could be used on the other buttons too. Or even go full out on overriding the .disabled class. pointer-events: none might be required to actually make the button unclickable.

Some lightly tested suggestions (the margin change on the label makes border-right on the left button show). I'm sure more improvements are possible (the color/box-shadow change seems unneccessary):

.repo-buttons .disabled-repo-button .label {
  opacity: .5;
}

.repo-buttons .disabled-repo-button a.button {
  opacity: .5;
  cursor: not-allowed;
  pointer-events: none;
}

.repo-buttons .disabled-repo-button a.button:hover {
  background: none !important;
  color: rgba(0,0,0,.6) !important;
  box-shadow: 0 0 0 1px rgba(34,36,38,.15) inset !important;
}

.repo-buttons .button > .label {
  border-left: none !important;
  margin: 0 !important;
}
Added @silverwind changes
Signed-off-by: jolheiser <john.olheiser@gmail.com>
@jolheiser

This comment has been minimized.

Copy link
Member Author

jolheiser commented Mar 3, 2019

I left pointer-events on because otherwise you lose the cursor and tooltip.
Clicking on it didn't seem to do anything in Chromium (which is intended behavior), however at the moment I cannot test on other browsers.

@silverwind

This comment has been minimized.

Copy link
Contributor

silverwind commented Mar 3, 2019

Almost there, but we need to modify the last style to make the inner border show:

.repo-buttons .ui.labeled.button > .label {
  margin: 0 !important;
}

Reason being the !important in Semantic here:

screenshot 2019-03-03 at 17 11 04

Fix border
Signed-off-by: jolheiser <john.olheiser@gmail.com>
@jolheiser

This comment has been minimized.

Copy link
Member Author

jolheiser commented Mar 3, 2019

Okay, updated. Thanks for the input, @silverwind
I can make UI changes, but I am definitely not a styling expert. 😅

@hexfs

This comment has been minimized.

Copy link

hexfs commented Mar 5, 2019

Conflict

@jolheiser

This comment has been minimized.

Copy link
Member Author

jolheiser commented Mar 5, 2019

Conflict

Resolved

@hexfs

This comment has been minimized.

Copy link

hexfs commented Mar 5, 2019

@GiteaBot GiteaBot added lgtm/done and removed lgtm/need 1 labels Mar 6, 2019

@techknowlogick techknowlogick merged commit 608781f into go-gitea:master Mar 6, 2019

2 checks passed

approvals/lgtm this commit looks good
continuous-integration/drone/pr the build was successful
Details

CL-Jeremy added a commit to CL-Jeremy/gitea that referenced this pull request Mar 12, 2019

Squashed commit of the following:
commit 5b33619
Author: Mike L <cl.jeremy@qq.com>
Date:   Tue Mar 12 20:17:26 2019 +0100

    Fix textarea also (to match body)

commit e76c56d
Author: Mike L <cl.jeremy@qq.com>
Date:   Tue Mar 12 19:41:39 2019 +0100

    Revert css temporarily to fix conflict

commit 6846c11
Author: Mike L <cl.jeremy@qq.com>
Date:   Tue Mar 12 19:17:38 2019 +0100

    Remove mistakenly introduced entry from .gitignore

commit 5ed4e51
Author: Mike L <cl.jeremy@qq.com>
Date:   Tue Mar 12 19:15:30 2019 +0100

    Tweak CJK, fix Yu Gothic, more monospace inherits

commit 26a460f
Merge: 125d1dc 7c20560
Author: Mike L <cl.jeremy@qq.com>
Date:   Mon Mar 11 13:51:33 2019 +0100

    Merge branch 'master' of https://github.com/go-gitea/gitea into issue-4173-alternative-fix

commit 125d1dc
Author: Mike L <cl.jeremy@qq.com>
Date:   Mon Mar 11 13:09:26 2019 +0100

    Add Lato for latin extd. & cyrillic, improve CJK

commit 7c20560
Author: GiteaBot <teabot@gitea.io>
Date:   Mon Mar 11 11:37:48 2019 +0000

    [skip ci] Updated translations via Crowdin

commit f9627ed
Author: MysticBoy <mysticboy@live.com>
Date:   Mon Mar 11 19:35:18 2019 +0800

    Update third-party-tools.en-us.md (go-gitea#6301)

    Add  Gitea Extension for Visual Studio

commit 4334fe7
Author: Lunny Xiao <xiaolunwen@gmail.com>
Date:   Mon Mar 11 11:44:58 2019 +0800

    update git vendor to fix wrong release commit id and add migrations (go-gitea#6224)

    * update git vendor to fix wrong release commit id and add migrations

    * fix count

    * fix migration release

    * fix tests

commit 2315019
Author: Jonas Franz <info@jonasfranz.software>
Date:   Mon Mar 11 03:54:59 2019 +0100

    Add support for client basic auth for exchanging access tokens (go-gitea#6293)

    * Add support for client basic auth for exchanging access tokens

    * Improve error messages

    * Fix tests

commit e0eb651
Author: GiteaBot <teabot@gitea.io>
Date:   Sun Mar 10 21:58:54 2019 +0000

    [skip ci] Updated translations via Crowdin

commit dbab98c
Author: zeripath <art27@cantab.net>
Date:   Sun Mar 10 21:56:36 2019 +0000

    Remove util.RemoveAll - should have been removed since go 1.7 (go-gitea#6299)

commit e836b88
Author: GiteaBot <teabot@gitea.io>
Date:   Sat Mar 9 21:18:31 2019 +0000

    [skip ci] Updated translations via Crowdin

commit f5cf9a8
Author: Aidan Fitzgerald <aidan-fitz@users.noreply.github.com>
Date:   Sat Mar 9 16:15:45 2019 -0500

    Copyedit docs (go-gitea#6275)

commit 8fffb06
Author: Jonas Franz <info@jonasfranz.software>
Date:   Sat Mar 9 17:29:58 2019 +0100

    Add regenerate secret feature for oauth2 (go-gitea#6291)

    * Add regenerate secret functionality

    * Fix lint

commit 8211e01
Author: John Olheiser <42128690+jolheiser@users.noreply.github.com>
Date:   Sat Mar 9 05:00:38 2019 -0600

    Add unit types to repo action URL to correctly show 404 when archived (go-gitea#6247)

    Signed-off-by: jolheiser <john.olheiser@gmail.com>

commit f7ffb19
Author: GiteaBot <teabot@gitea.io>
Date:   Fri Mar 8 20:28:33 2019 +0000

    [skip ci] Updated translations via Crowdin

commit 96f1720
Author: techknowlogick <matti@mdranta.net>
Date:   Fri Mar 8 15:25:47 2019 -0500

    Use golang 1.12 to build in dockerfile (go-gitea#6285)

commit 5c69e31
Author: John Olheiser <42128690+jolheiser@users.noreply.github.com>
Date:   Fri Mar 8 12:15:46 2019 -0600

    Add security note to issue template (go-gitea#6281)

commit 062de8e
Author: GiteaBot <teabot@gitea.io>
Date:   Fri Mar 8 17:43:26 2019 +0000

    [skip ci] Updated translations via Crowdin

commit bd4be43
Author: John Olheiser <42128690+jolheiser@users.noreply.github.com>
Date:   Fri Mar 8 11:42:59 2019 -0600

    Third party docs (go-gitea#6282)

commit 489419c
Author: GiteaBot <teabot@gitea.io>
Date:   Fri Mar 8 16:45:46 2019 +0000

    [skip ci] Updated translations via Crowdin

commit e777c6b
Author: Jonas Franz <info@jonasfranz.software>
Date:   Fri Mar 8 17:42:50 2019 +0100

    Integrate OAuth2 Provider (go-gitea#5378)

commit 9d3732d
Author: Antoine GIRARD <sapk@users.noreply.github.com>
Date:   Fri Mar 8 02:54:10 2019 +0100

    [Contrib] Checkout a PR (go-gitea#6021)

commit 9fd8b26
Author: techknowlogick <matti@mdranta.net>
Date:   Thu Mar 7 16:30:25 2019 -0500

    Add robots.txt as reserved username (go-gitea#6272)

    Fix go-gitea#6271

commit f2de5dc
Author: mrsdizzie <joe.mccann@gmail.com>
Date:   Thu Mar 7 15:12:01 2019 -0500

    Replace linkRegex with xurls library (go-gitea#6261)

    * Replace linkRegex with xurls library

    Rather than maintaining a complicated regex to match URLs for
    autolinking, gitea can use this existing go library that takes care of
    the matching with very little code change to gitea itself. After
    spending a while trying to find the perfect regex for all cases this library
    still works better as it is more flexible than a single regex ever will be.

    This will also fix the following issues: go-gitea#5844 go-gitea#3095 go-gitea#3381

    This passes all our current tests and I've added new ones mentioned in
    those issues as well.

    * Use xurls.StrictMatchingScheme instead of xurls.Strict

    This is much faster and we only care about https? links to preserve
    existing behavior.

commit 01bd1fc
Author: GiteaBot <teabot@gitea.io>
Date:   Thu Mar 7 19:16:42 2019 +0000

    [skip ci] Updated translations via Crowdin

commit 020075e
Author: mrsdizzie <joe.mccann@gmail.com>
Date:   Thu Mar 7 14:13:44 2019 -0500

    Remove visitLinksForShortLinks features (go-gitea#6257)

    The visitLinksForShortLinks feature would look inside of an <a> tag and
    run shortLinkProcessorFull on any text, which attempts to create links
    out of potential 'short links' like [[test]] [[link|example]] etc...
    This makes no sense because you can't have nested links within an <a>
    tag. Specifically, the html5 standard says <a> tags can't include
    interactive content if they contain the href attribute:

     http://w3c.github.io/html/single-page.html#the-a-element

    And also defines an <a> element with a href attribute as interactive:

     http://w3c.github.io/html/single-page.html#interactive-content

    Therefore you can't really put a link inside of another link. In
    practice none of this works anyways since browsers won't render it, it
    would probably be broken if they tried, and it is causing a bug
    (go-gitea#4946). No current tests rely on this behavior either.

    This removes the feature and also explicitly excludes the
    current visitNodeForShortLinks from looking in <a> tags.

commit ad86b84
Author: GiteaBot <teabot@gitea.io>
Date:   Wed Mar 6 00:50:36 2019 +0000

    [skip ci] Updated translations via Crowdin

commit 608781f
Author: John Olheiser <42128690+jolheiser@users.noreply.github.com>
Date:   Tue Mar 5 18:48:30 2019 -0600

    Fix fork button (go-gitea#6223)

commit 6460cff
Author: GiteaBot <teabot@gitea.io>
Date:   Tue Mar 5 20:18:01 2019 +0000

    [skip ci] Updated translations via Crowdin

commit f80caa5
Author: Zsombor <gzsombor@users.noreply.github.com>
Date:   Tue Mar 5 21:15:24 2019 +0100

    Fix go-gitea#6234 : Check organization visibility before everything else (go-gitea#6235)

    * Fix go-gitea#6234 : Check organization visibility before everything else

    * Ensure that Owner is available in the Repo

commit b257e04
Author: stevegt <stevegt@t7a.org>
Date:   Tue Mar 5 15:39:41 2019 +0100

    Add ability to sort issues by due date (go-gitea#6206) (go-gitea#6244)

    Signed-off-by: Steve Traugott <stevegt@t7a.org>

commit 4512634
Author: Muhammed TİFTİKÇİ <muhammedtiftikci@outlook.com>
Date:   Tue Mar 5 06:13:51 2019 +0300

    Make organization dropdown scrollable when using mouse wheel (go-gitea#5988)

    * Fix go-gitea#5580

commit f066bd2
Author: zeripath <art27@cantab.net>
Date:   Tue Mar 5 02:52:52 2019 +0000

    Prevent double-close of issues (go-gitea#6233)

commit 1986269
Author: Maurizio Porrato <maurizio.porrato@gmail.com>
Date:   Tue Mar 5 02:34:52 2019 +0000

    Override xorm type mapping for U2F counter (go-gitea#6232)

commit 141c58f
Author: Lanre Adelowo <adelowomailbox@gmail.com>
Date:   Sun Mar 3 23:57:24 2019 +0100

    add isAdmin to user model (go-gitea#6231)

    update vendor and add tests

    fix swagger

@jolheiser jolheiser deleted the jolheiser:6222_fix_fork_button branch Mar 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.