UX: move templates to main LLM config tab, restyle #813
Conversation
assets/javascripts/discourse/components/ai-llms-list-editor.gjs
Outdated
Show resolved
Hide resolved
assets/javascripts/discourse/components/ai-llms-list-editor.gjs
Outdated
Show resolved
Hide resolved
assets/javascripts/discourse/components/ai-llms-list-editor.gjs
Outdated
Show resolved
Hide resolved
| <LinkTo | ||
| @route="adminPlugins.show.discourse-ai-llms.show" | ||
| class="btn btn-default" | ||
| @model={{or llm.id llm.llm.id}} |
There was a problem hiding this comment.
After first configuring an LLM, when you save and it returns you to this list page the model is structured llm.llm.id and not llm.id so this avoids the error (which existed before this PR)
this fix doesn't seem ideal though, so if anyone has a suggestion I'd appreciate it!
There was a problem hiding this comment.
I can't check code locally for now, but I looked at the code quickly.
Can you show me what console.log(this.args.model) gives at this line https://github.com/discourse/discourse-ai/blob/main/assets/javascripts/discourse/components/ai-llm-editor-form.gjs#L95 please?
There was a problem hiding this comment.
Maybe I borked the routing a bit when I separated that LLM route? Here is what the console.log gives:
There was a problem hiding this comment.
I fixed this in my commit ... honestly I think it may be an issue with llms hallucinating code cause it had ai_persona there ... which is totally unrelated.
|
looking at this at the moment and cleaning some stuff (some issues around translation) |
| {{/if}} | ||
| <section class="ai-llms-list-editor__templates"> | ||
| <div class="admin-page-subheader"> |
There was a problem hiding this comment.
Please use the actual AdminPageSubheader component from core here, here is an example:
Docs are at https://meta.discourse.org/t/creating-consistent-admin-interfaces/326780
| </div> | ||
| </div> | ||
| <div class="ai-llms-list-editor__templates-list"> | ||
| {{#each this.preConfiguredLlms as |llm|}} |
There was a problem hiding this comment.
Would really prefer these "section landing" tiles are reusable components, this is why I opened this draft PR https://github.com/discourse/discourse/pull/28477/files . At the least could you please extract this to a component in this PR then we can replace with a core version later?
There was a problem hiding this comment.
lets do this in a followup, really want to land this. I addressed the rest of the feedback.
assets/javascripts/discourse/components/ai-llms-list-editor.gjs
Outdated
Show resolved
Hide resolved
assets/javascripts/discourse/components/ai-llms-list-editor.gjs
Outdated
Show resolved
Hide resolved
Fix list to handle models with no description Remove hacky Or Correct spec which was not confirming shape
Overloaded H3 semantics is a bit confusing, but we get to use the component.
|
@martin-brennan fyi one little glitch here is that @awesomerobot wanted the element in the subheader to be an h2 ... I worked around it but now semantically stuff is a bit odd. Not the end of the world but worth noting. |
|
@SamSaffron @martin-brennan ideally headings should always be sequential and we shouldn't be going from H1 to H3 on any pages If H2 feels too large, we should adjust the size in the CSS rather than skipping sequence |
|
@awesomerobot @SamSaffron will bring that feedback into dev for Ella. And I will make the section landing item component in core and use it here for the template list 👍 |
|
I have a PR now to use the section components from core #817 |
This updates the layout and styles of /admin/plugins/discourse-ai/ai-llms
These updates simplify the process a bit by showing the templates initially, and allows for more space to include details like the model description.
Before:
After: