Open the corresponding tab when a server is clicked in settings page

[yuya-oc]

Before submitting, please confirm you've

• read and understood our Contributing Guidelines
• completed Mattermost Contributor Agreement
• executed npm run lint:js for proper code formatting

Please provide the following information:

Summary
The part 2 of #400.

Proposal:
To prevent unexpectedly clicking "Edit" or "Remove", a clickable area is limited to server name and URL. (Red squares in the screenshot)

Issue link
#400

Test Cases

1. Open the settings page.
2. Click a server.
3. The page goes back to the main page and the corresponding tab is opened.

Additional Notes
Limitation: Currently the changes in settings page would be lost when this feature performs. There is no way to know whether the changes are already saved.

[jasonblais]

Thanks @yuya-oc!

Limitation: Currently the changes in settings page would be lost when this feature performs. There is no way to know whether the changes are already saved.

I see -- due to this limitation, I hit the following error:

1. Add a new server
2. Click the newly-added server

Observed:

The app attempts to open the tab, but gives a blank page since the server wasn't saved

I wonder if we need to implement auto-saving on the Settings page first, or as part of this feature..that way the user wouldn't have to hit "Save" each time they change something, and it would resolve this limitation.

[yuya-oc]

Ah, I missed this case. How about saving the config when this feature performs? It's not hard.

[jasonblais]

That could work -- would saving App options be included as part of this, or would it be something different?

[yuya-oc]

For now, app options will be included.

[yuya-oc]

Now the settings are saved as well as "Save" when a server is clicked. https://circleci.com/gh/yuya-oc/desktop/148#artifacts

This fixes the issue, #425 (comment)

[jasonblais]

Thanks @yuya-oc!

Just a couple of minor notes:

1. Can we extend the clickable area as shown below?

• Maximize the area so there is no wasted space [I agree on leaving some space so you don't misclick when trying to edit a server]
• Make the area consistent across servers. For instance, if I hover over pre-release in the screenshot, then move my mouse down to the next server, I'd expect the area still be clickable

2. Can we add an on_hover effect when your mouse is over the clickable area? Something similar to when you hover over the tabs.

This is to follow the Mattermost UX guidelines

3. Now that settings and servers auto-save, and you can access the servers by clicking them, would we be able to remove "Cancel" and "Save" buttons from the bottom of the Settings page? They've always been a bit confusing.

If we do this, we can make the "X" have the same action as "Save".

One concern I have is that users cannot "undo" their actions, but I think that's fine (it's easy to edit/remove servers and people can update the app options easily).

[yuya-oc]

https://circleci.com/gh/yuya-oc/desktop/154#artifacts

• 1 and 2: I could implement them. Thanks for mention!
• 3: Technically possible. But I'm not sure whether it's reasonable that there are no Save/Cancel buttons. Does "confusing" mean that the clicking on the server list implicitly saves the config without clicking "Save"? If it does, I agree.

[jasonblais]

Yeah exactly: since the

• Settings page saves automatically, and
• you can access the server by clicking the list, or the "x" button (which should then act like the save button),

the Cancel/Save buttons are redundant.

It's also not obvious that the Cancel/Save buttons are both for the server management and app options --> I would assume that servers save automatically, and might end up clicking "Cancel" just to undo my changes to the app options.

NOTE: I will actually confirm this with the UX design team today (we have a meeting in about three hours), and I'll let you know if they feel the same.

[yuya-oc]

I see.

BTW I noticed that 1.) made a good side effect. Now the layout of server list is not broken when the window width is too narrow :)

[jasonblais]

Hooray! 🎉

[yuya-oc]

@jasonblais Should we remove the Cancel/Save buttons?

[jasonblais]

Hey @yuya-oc, really sorry for not replying back.

Yes, removing the "cancel/save" buttons was approved, but it was also proposed that we add some type of feedback to let the user know that settings were saved. Here's a link to a brief specification of its design. It's subject to change as we might update the design a little (we'll be finalizing it over the next couple of days).

How much work would you estimate it would take to add feedback as described on the spec? Would it be reasonable for v3.6?

That would be one of the last things for v3.6, apart from testing #428, part 3 of #401 and having Asaad's help with the plus-icon in #401.

[yuya-oc]

Probably 1 day after the appearance is decided once.

I think there are three choices for the indicator level of this purpose.

1. For the whole of settings page.
2. For each section
3. For each item

This is just a mock of 2 and 3 for proposal. If 1 is chosen, something like toastr would make sense.

[yuya-oc]

Now all options are automatically saved. https://circleci.com/gh/yuya-oc/desktop/162#artifacts

[jasonblais]

Awesome! I'll test this out soon

[jasonblais]

Thanks @yuya-oc! This is wonderful work 🎉

Just a few notes

A. "Saved!" appears after cancelling server updates

1 - Go to File > Settings and select "Add new server"
2 - Hit "Cancel"

Observed: "Saved!" appears

Expected: No feedback since I made no updates

---

B. "Saving..." doesn't appear for 0.5 seconds

I think the spec proposed having "Saving..." appear for 0.5 seconds after the most recent change on the Settings page, before "Saved!" is shown.

Example scenario:

1 - Save a setting at t = 0s
- "Saving..." appears
2 - Save a setting at t = 0.3s
- "Saving..." continues
3 - Save a setting at t = 0.5s
- "Saving..." continues
- at t = 1s, "Saved!" appears

^ This is so that it's more clear that
a) the settings are saved after steps 2 and 3 and
b) the transition between "Saving..." and "Saved!" isn't too quick

---

C. Question: Does the fading out take a second?

It appears to happen a bit faster (maybe in 0.5 seconds), but I might be wrong. It felt a little abrupt for me.

---

D. Location of "Saved!" indicator

You've correctly implemented the location of the indicator, but I think it's not going to work for all screen sizes..see attached for an example

Hence, propose:

1. moving "Saving..." / "Saved!" indicator to the header where the "Settings" title appears, left-aligned with the "Server Management" title

2. Change the text Sorry something went wrong. Please check your connection and try again to Can't save your changes. Please try again. so that it fits in top left corner of any window 800x600 or bigger, without overlapping "Settings" title.

---

E. Style of "Saved!" indicator

Sorry, one more change from the original spec:

1 - please help remove "!" from "Saved!". I forgot we had a guideline not to end feedback messages in an exclamation mark.

2 - please help change the colour for "Saving..." and "Saved!" indicators to match the original "Save" button

This style is more consistent with what we use elsewhere, such as the "Add new server" modal

[jasonblais]

F.. I'm getting a bunch of JavaScript error messages upon refresh.

Nothing on the UI, just on the developer console

I'm not sure if they're from this PR or from others, but reporting here since they appeared while testing on this PR's artifact.

EDIT: This is on the pre-release server

[yuya-oc]

Thanks for feedback. This is the latest build: https://circleci.com/gh/yuya-oc/desktop/164#artifacts

A:
Fixed. Auto-saving was accidentally working.

B:
Fixed. I misunderstood about 0.5 seconds specs.

C:
The fading out is defined as easeOutExpo for 1 second. easeOutExpo quickly changes its state at first, so the indicator would almost disappear in the first half of the animation. As the result, you would feel the duration is shorter than 1 second.

D:
Fixed.

E:
I'm not sure why the color makes consistent. The indicator is not a button, so I feel we don't have to change.

F:
I had to check it in #415. The messages are not related to servers. Certainly they are actually errors though, but the feature is correctly working.

[jasonblais]

Thanks! Testing now

[jasonblais]

C - Sounds good, wanted to confirm

E - Let's leave the colour as-is

F - Should we try and clear these errors for 3.6? We should try to keep the JavaScript errors at a minimum.

Clearing these errors would be okay next week, they don't need to be included in the first release candidate cut.

PS: I shared two videos with you on pre-release as I wasn't able to upload them here. They show two potential issues to investigate, but otherwise all look great!

Wonderful work @yuya-oc :)

[yuya-oc]

F:
If possible, we should fix. But just for now, I'm not sure whether I can fix it in RC period. How do you feel, @jnugh?

[jasonblais]

F: I can create a separate ticket for it and we can fix it in a later version, that works too

[yuya-oc]

Thanks @jasonblais. If it seems possible, I will try that in RC.

Then, this is the latest build to fix animation issues, and it's rebased against to the latest master. https://circleci.com/gh/yuya-oc/desktop/166#artifacts

PS: I shared two videos with you on pre-release as I wasn't able to upload them here. They show two potential issues to investigate, but otherwise all look great!

It seems the point of issues is that the fading out is not cancelable in ReactCSSTransitionGroup. So I rewrited it with a simple css transition.

[jasonblais]

Thanks @yuya-oc! The latest changes are working as expected, great work!

Created an issue for the JavaScript errors in #440 and another unrelated issue in #439.