Closed RouxRC closed 6 years ago
Fixed by ec36755 and 63d33fb. Default values (sort by status, grouped by article, read mode off) don't change the URL.
Note that read mode affects all modules, not only amendements.
Having readmode active for all pages results in breaking urls in Articles view since we don't have a url argument for the selected steps and article yet (urls such as articles.html?loi=pjl15-republique_numerique&read=1 will open on an open sidebar with only documentation inside which sounds problematic (and is not possible to activate normally until an article has been clicked))
So I don't feel like this can be put in production until then :/
In the mean time, maybe add a check that we are on amendements mode only for the readmode part or simply comment the viewmode url in the code until fixed in articles view
BTW: since we're urlizing all these arguments, diffMode in articles view should definitely also be part of it PS: Yes I know this needs to wait for mod1 to be better refactored :/
Another thing: currently navigating with the browser's previous/next page functionalities does not take these new arguments into account (meaning changing the amendment selected and displayed in the sidebar for instance or the view choice), not sure which is the best angular way to tell it to watch the location for those, maybe in the mainCtrl ? ping @fmassot
I found a solution to handle URL changes on history back/forward, but now the problem lies in deciding which parameters should be handled that way.
For example with amendments, sort order and grouping mode are saved as querystring parameters, but if the user changes it multiple times we probably don't want that to add browser history entries.
So my proposal is to only allow history entries for parameters that change what is displayed, not how it is displayed, ie for amendments:
What do you think ?
I discussed it briefly with our interface designer at médialab, we agreed that we prefer to make it consistent and apply it for everything: as long as something gets into the url, it should get into the history
So we get rid of sorting/viewmode in the url then ? Otherwise the behavior will not be very consistent (eg. Open amendments for a step, change sorting a bunch of times, switch to a different step, change sorting.... Will make you have to pass through all display changes to go back to 1st step). Usually history movements does not affect display options IMO.
I think the contrary: we definitely want these options in the urls for permalinks, and as long as something is in the url it should be reflected in history (even anchors behave like this on classical web) Open debate @fmassot @teymour etc. !
A quick remark though: the way I did it in amendments, view options are saved in the url. But they replace the last history entry. That way, when you go back in history, you don't have to click through all sort/group/etc changes, but the view options you had when you were there last are restored.
For example if you do the following actions:
Only 2 history points are saved (besides current url)
IOW, only points where what was displayed changed, each saved with the last display options that were active when you were there. At any time, the url still reflects exactly what you see and can be used to restore the exact same view (except for scrolling positions)
I like the rule "everything in the url goes into the history" because this rule is general and simple. As a user, I don't really want to know if a url parameter is taken or not into the browser history. If a user need to click 70x on the back button to navigate correctly, there is something wrong and maybe some url parameters are useless or just add some noise, so we need to remove them.
to allow links to specific views :
will have to do further on for other viz, but we can already go for it in amendments
edit: maybe keep no argument when default values chosen to avoid too long default urls? (note for later: we need to get rid of those steps name in urls that are so long, a number shoul suffice, full names can be inferred if necessary from stepsbar data)