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

improve handling of noscript (related to #349) #408

Merged
merged 1 commit into from
Feb 6, 2019

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Jan 29, 2019

This is a rough attempt at improving the behavior of cla-assistant as seen by browsers that render <noscript>

Currently the rendering looks like this:
image

image

(That the username disappears long before the brand logo is a little odd, but we can cross the mobile bridge later.)

@CLAassistant
Copy link
Member

CLAassistant commented Jan 29, 2019

CLA assistant check
All committers have signed the CLA.

@jsoref
Copy link
Contributor Author

jsoref commented Jan 29, 2019

As an aside, when I do enable JS, I get:

Failed to load resource: the server responded with a status of 401 ()

for /api/github/call:

{obj: "users", fun: "get", arg: {}}
arg: {}
fun: "get"
obj: "users"

Which results in the Hey message disappearing:

<div ng-show="!!user.value.login" class="ng-hide">
                    <a target="spcae">
                        <img style="height:25px;">
                        </a><a class="navbar-text-hide">&nbsp; Hey,
                            <b class="ng-binding"></b>!</a>
                    
                </div>

@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.013% when pulling 9780f27 on jsoref:issue-349 into 64edc29 on cla-assistant:master.

@KharitonOff
Copy link
Contributor

Thank you very much for looking into it, appreciate your work! But this wouldn't solve the general problem that the whole application is written in JS and wouldn't work in 'no JS'-mode anyway.

@jsoref
Copy link
Contributor Author

jsoref commented Feb 1, 2019

@KharitonOff : the message can be adjusted...

@KharitonOff
Copy link
Contributor

KharitonOff commented Feb 1, 2019

Ok, I see. If we make it clear in a <noscript> tag that CLA assistant needs JS in order to work properly, then it makes sense for me.

@jsoref
Copy link
Contributor Author

jsoref commented Feb 1, 2019

If you want to write that text, I can integrate it, or you can merge this and work from it.

@KharitonOff KharitonOff merged commit acab844 into cla-assistant:master Feb 6, 2019
@jsoref jsoref deleted the issue-349 branch February 6, 2019 16:09
KharitonOff added a commit that referenced this pull request Feb 18, 2020
improve handling of noscript (related to #349)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants