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

Add What's new modal #372

Merged
merged 1 commit into from
Feb 23, 2021
Merged

Add What's new modal #372

merged 1 commit into from
Feb 23, 2021

Conversation

bagage
Copy link
Collaborator

@bagage bagage commented Feb 22, 2021

This PR adds a new modal that is displayed to users when they come from an earlier version (that information being stored in localStorage). What's new? modal is also always accessible in About modal, to see the whole history.

For instance assuming I previously was on 0.13.1, on next launch I will see:

Capture d’écran de 2021-02-22 15-46-56

There are some facts I'm not totally happy with:

  • the library I'm using to parse the changelog messes the markdown up quite a bit (links and brackets mainly)
  • the changelog is in English only (but I don't think it needs to be translated)

Anyway any feedbacks are welcome as always :).

Copy link
Owner

@nrenner nrenner left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, I recently thought it would be nice to have some kind of update notification in the client.

I guess we can live with english only and messed up markdown for now (open issue?). Don't know if it would be an alternative to use a proper markdown to HTML renderer and in the client search for the version and split the HTML there ourselves.

But I do hate sites that shove a popup in my face when entering, no matter for what reason. So, could you please change this to something less obstrusive, e.g.

  • expand Message class with a showInfo type to show a short message and a link to open the modal. Such a message doesn't get in the way and does not require interaction as it disappears with the next route request
  • link the version in the Navbar to open the modal and add some "new" badge and highlight

@bagage
Copy link
Collaborator Author

bagage commented Feb 23, 2021

I modified the PR according to your feedback @nrenner, thanks!
I dropped the changelog-parser dependency in favor of the more popular marked, since we don't need much parsing actually it should be OK.

One thing I did not do is having a link directly from navbar to Whatsnew modal: when you click on the version, it opens the About modal yet (from which you can go to What's new modal). I wasn't sure about how to do that cleanly and comprehensible by users, so I left it as is. Feel free to modify it if you have an idea for that!

@nrenner nrenner merged commit 1a695db into master Feb 23, 2021
@nrenner
Copy link
Owner

nrenner commented Feb 23, 2021

That's fine, it was more like an either/or suggestion.

Thanks!

@nrenner nrenner mentioned this pull request Mar 22, 2021
@nrenner nrenner added this to the 0.16.0 milestone Mar 25, 2021
stefankeidel added a commit to cxberlin/brouter-web that referenced this pull request Apr 8, 2021
* upstream/master:
  Update France Go area to 10km
  Update bootstrap-select dep to avoid error on mobile
  Improve modal dialogs consistency
  Rename dialog
  Make modals fullscreen on mobile
  Hide expand icon on moblie (nrenner#296)
  Gray out unselectable layers (nrenner#381)
  Update translations
  Translate overlay type and fix width computation (nrenner#379)
  Use plural form when multiple overlays are active (nrenner#378)
  Add message if no elevation data is available (nrenner#373)
  Add missing translatable content (nrenner#376)
  Remove debug code
  Update translations
  Add What's new modal (nrenner#372)
  Add Babel and core-js & regenerator polyfills (nrenner#367)
@nrenner nrenner deleted the feat/whatsnew branch July 18, 2022 14:24
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.

2 participants