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

Use web component for header and footer. Add extra links to header #150

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

halkeye
Copy link
Member

@halkeye halkeye commented Oct 17, 2022

image

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

The additional header links aren't very obvious to me

image

navigating with them feels strange, tbh I preferred the second nav bar more but maybe there's a better way to do it, thoughts?

Comment on lines -53 to -54
implementation("org.webjars:jquery:3.6.1")
implementation("org.webjars:jquery-ui:1.13.2")
Copy link
Member

Choose a reason for hiding this comment

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

one of these changes causes a browser console error, I assume the version of bootstrap relies on this (even if we aren't using that)

image

</j:if>
<jio-navbar-link href="/myself">Profile</jio-navbar-link>
<jio-navbar-link href="/logout">Logout <ion-icon name="log-out-outline" /></jio-navbar-link>
Copy link
Member

Choose a reason for hiding this comment

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

the ion-icon doesn't seem to be showing for me

image

@halkeye
Copy link
Member Author

halkeye commented Oct 18, 2022

navigating with them feels strange, tbh I preferred the second nav bar more but maybe there's a better way to do it, thoughts?

Yea I can't think of anything better. I tried putting a divider in between, but it didn't look much better. It just felt weird.

Any idea what the old one looked like?

@timja
Copy link
Member

timja commented Oct 18, 2022

Not sure what the original one did,

it's currently like:
image

which is far better than the broken one was that was in place for years

I wonder if it would work if you got rid of all the other links, (bar maybe one or two that take you to jenkins.io and plugin site)

@halkeye
Copy link
Member Author

halkeye commented Oct 18, 2022

Oh I like that one. A simple flexbox or something. What I saw when I logged in was a broken collapse bar.

@timja
Copy link
Member

timja commented Oct 18, 2022

played around with less links in this:
image

not sold on the separator but I think it needs something

Oh I like that one. A simple flexbox or something. What I saw when I logged in was a broken collapse bar.

I fixed it in the weekend, it was using bootstrap 3 classes, when jenkins.io had upgraded to bootstrap 4...

@dduportal
Copy link
Contributor

Comment on lines +67 to +68
<script async="" data="jio" src="https://unpkg.com/@halkeye/jenkins-io-components?module" type="module" />
<script async="" data="jio" nomodule="" src="https://unpkg.com/@halkeye/jenkins-io-components" />
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<script async="" data="jio" src="https://unpkg.com/@halkeye/jenkins-io-components?module" type="module" />
<script async="" data="jio" nomodule="" src="https://unpkg.com/@halkeye/jenkins-io-components" />
<script async="" data="jio" src="https://unpkg.com/@jenkinsci/jenkins-io-components?module" type="module" />
<script async="" data="jio" nomodule="" src="https://unpkg.com/@jenkinsci/jenkins-io-components" />

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants