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.

Sign up for GitHub

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

Show namespace.Code in chooser #623

Merged
merged 8 commits into from
Nov 22, 2023
Merged

Show namespace.Code in chooser #623

Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
411ea0b
Show namespace.Code in chooser
suprjinx Nov 20, 2023
50230ea
Merge branch 'main' into show-code-on-chooser
suprjinx Nov 20, 2023
b0d41a5
move `code - description` format to model
suprjinx Nov 20, 2023
00b8e19
Merge branch 'show-code-on-chooser' of github.com:suprjinx/fasttrackm…
suprjinx Nov 20, 2023
8f4e5c1
Merge branch 'main' into show-code-on-chooser
suprjinx Nov 21, 2023
d1fc8e9
PR change requests
suprjinx Nov 21, 2023
28bf15f
Fix errant tab chars and rename element id
suprjinx Nov 22, 2023
62ed780
Merge branch 'main' into show-code-on-chooser
suprjinx Nov 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions pkg/ui/chooser/embed/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,18 +76,25 @@
<p>This is the modern Aim UI, much faster than MLFlow.</p>
</article>
</div>
<p>Selected namespace: <b><u>{{.CurrentNamespace.Description}}</u></b></p>
<p>Selected namespace:
<b>
<u>{{.CurrentNamespace.Code}}</u>
{{ if ne "" .CurrentNamespace.Description }}
- {{ .CurrentNamespace.Description }}
{{ end }}
Copy link
Member

Choose a reason for hiding this comment

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

I think we should apply this logic everywhere for consistency, probably by computing this before passing it to the template (as e.g. DisplayName)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

afaik this is the only place -- it looked strange to do this in the listing because the lengths were so different

Copy link
Member

Choose a reason for hiding this comment

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

But the listing is probably where the description is the most useful if we display it, no?
I think we either don't display the description in the chooser at all, or we display it everywhere in the chooser.

Copy link
Member

Choose a reason for hiding this comment

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

I guess it looked strange because the namespaces are displayed centred, but this might also be an issue when you have codes of various lenghts. We may want to align this all differently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes left-align will probably look better, i'll try it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image
image

Copy link
Member

Choose a reason for hiding this comment

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

could you please make the text left-aligned and move the arrow to the left of it?
also, I think it might look better with the description in parentheses, and with the default namespace being described as "Default namespace".

e.g.

  custom 1
  custom2 (has description)
→ default (Default namespace)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how's this?
image

</b>
</p>

<ul id="namespaces-list">
{{ range .Namespaces }}
{{ if ne .Code $.CurrentNamespace.Code }}
<li>
<a href="#" onclick="window.location = window.location.origin + {{if eq .Code "default"}}'/'{{else}}'/ns/{{.Code}}/'{{end}}">{{.Description}}</a>
<a href="#" onclick="window.location = window.location.origin + {{if eq .Code "default"}}'/'{{else}}'/ns/{{.Code}}/'{{end}}">{{.Code}}</a>
</li>
{{ else }}
<li class="selected-namespace">
<i id="current-icon" class="Icon__container icon-long-arrow-right"></i>
<b id="current-namespace">{{$.CurrentNamespace.Description}}</b>
<b id="current-namespace">{{$.CurrentNamespace.Code}}</b>
</li>
{{ end }}
{{ end }}
Expand Down