Skip to content

Move app info to version modal#89

Merged
gfwilliams merged 7 commits into
espruino:masterfrom
RKBoss6:AppLoaderInfo
Jun 4, 2026
Merged

Move app info to version modal#89
gfwilliams merged 7 commits into
espruino:masterfrom
RKBoss6:AppLoaderInfo

Conversation

@RKBoss6

@RKBoss6 RKBoss6 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Made the changes from #88 on a new branch, and found it easier to open a new PR. This moves the app info segment to the top of the version modal, found when clicking the version for both mobile and desktop. It also reverses the changelog order so users can see both the app info and the most recent version as soon as it opens.

Copilot AI review requested due to automatic review settings June 2, 2026 01:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR updates the app version UI to open a consolidated “App Information” prompt and refactors changelog rendering to better handle multi-line entries.

Changes:

  • Replace version “ChangeLog” links with “App Information” links that include app details + changelog.
  • Refactor changelog parsing/rendering (group multi-line entries, reverse by entry, highlight installed version).
  • Introduce getAppInfo to generate app metadata text for tooltips and expanded info prompts.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
js/utils.js Updates version link click handler to call showAppInfo instead of showChangeLog.
js/index.js Adds app info prompt logic, refactors changelog loading/rendering, and introduces getAppInfo.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread js/index.js Outdated
Comment thread js/index.js Outdated
Comment thread js/index.js Outdated
Comment thread js/index.js
Comment thread js/index.js
@RKBoss6

RKBoss6 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

As for the report app issue button, I'd like it to be inline with the Ok button in the prompt for now, so it looks cleaner and is always visible, but that'll require some tinkering with the showPrompt function itself... I don't want to do a button in the modal html because it'll be affected by scrolling, will look misaligned with the ok button, etc. @gfwilliams what's your thoughts on this?

@RKBoss6 RKBoss6 mentioned this pull request Jun 2, 2026
@gfwilliams

Copy link
Copy Markdown
Member

Maybe let's leave the 'report app issue' button for a separate PR to keep this one manageable? But yes, we could add an extra line for 'buttons.reportissue' to https://github.com/espruino/EspruinoAppLoaderCore/blob/master/js/ui.js#L117 - it's probably easier than trying to allow totally custom buttons.

There's an awful lot of code there for the ChangeLog when we basically just had contents = lines.join("<br>"); before. Is that really needed? It just feels like a lot of potential for bugs for something that's only really going to be glanced at (and it's not hard to see what the currently installed version is)

@RKBoss6

RKBoss6 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Yeah, it's a lot - but the way it works is it finds all the version headers (0.24), and then groups it into blocks until the next header, so that way we're able to reverse the block order and keep the lines underneath, eg:

0.34: Fix xxx
      Change xxx
      Modify xxx

Then it just applies a bolded format to the headers for visibility.

I'll work on simplifying the logic out, but I'd like to keep the reversal of the changelog, as that's pretty much the only way to make sure that users see the latest version and the app info without feeling janky or unfinished to use (keeping the app info on the bottom doesn't really seem that polished, as there's no way to indicate the upper lines are changelog and then go into the app info... Even though it's a small, glanceable menu that people won't spend more than a minute on, making sure people can see everything they want to at a first glance is pretty important, and with this, you see app info, and the changelog, neatly split with a "ChangeLog" header so people know what they're looking at. What's your thoughts on this? If worst comes to worst, we could just append the app info at the bottom, but I like the look of it now much, much more.

@gfwilliams

Copy link
Copy Markdown
Member

Thanks - right, I see what you mean about the ChangeLog ordering...

Please could you try and get rid of some of the indenting warnings the linter is throwing up on e3d5bb1?

And while it's not really your code, since Copilot spotted it, it would be good to move the percent+percentText calculations inside that if statement before infoTxt.push so avoid a divide by 0.

But otherwise I guess we're good to merge?

@RKBoss6

RKBoss6 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Should be good to go now!

@gfwilliams

Copy link
Copy Markdown
Member

Looks great - thanks!

@gfwilliams gfwilliams merged commit c99fc87 into espruino:master Jun 4, 2026
1 check passed
gfwilliams added a commit to espruino/BangleApps that referenced this pull request Jun 4, 2026
gfwilliams added a commit to espruino/BangleApps that referenced this pull request Jun 4, 2026
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.

3 participants