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

CODENVY-1579: show server's address along with URL #3866

Merged
merged 1 commit into from
Mar 10, 2017
Merged

Conversation

akurinnoy
Copy link
Contributor

@akurinnoy akurinnoy commented Jan 24, 2017

Signed-off-by: Oleksii Kurinnyi okurinnyi@codenvy.com

What does this PR do?

This PR improves Service section of machine config to show server's address along with URL.

che-workspace-running-runtime-servers

What issues does this PR fix or reference?

codenvy/codenvy#1579

Changelog

Show server's address along with URL.

Release notes

  • Fixed displaying both server's URL and address for runtime and undefined protocol

Docs Pull Request

eclipse-che/che-docs#93

@ashumilova ashumilova added the kind/enhancement A feature request - must adhere to the feature request template. label Jan 24, 2017
@ashumilova ashumilova added this to the 5.2.0 milestone Jan 24, 2017
@codenvy-ci
Copy link

Copy link
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

nice !

@gazarenkov
Copy link
Contributor

Do we really need two lines row (address and URL)? Note, there will be Address for all the Servers.
So, why not to make it as usual with 2 columns with clear titles?
(Column name "Runtime" sounds about nothing)

@TylerJewell
Copy link

TylerJewell commented Jan 24, 2017

I agree. @gazarenkov - is the URL part even needed? I think most people can put together the protocol and address to figure it out. It's just redundant data at that point.

@gazarenkov
Copy link
Contributor

Yes.
It is ready to use URL, including path (can be added to the Server definition too btw). So

  • it is just convenient to not to concatenate it every time on client
  • protocol, host and port may not be enough (especially if URL is modified due to proxy)
  • moreover in a case of infra under reverse proxy (such as codenvy.io) it may be even impossible to to form right external URL since host and path may be not the same

@akurinnoy
Copy link
Contributor Author

@gazarenkov We did it because otherwise we would run into this problem:

screenshot

@slemeur
Copy link
Contributor

slemeur commented Jan 25, 2017

Yes so basically, we are going to have space problems. A lot of user would have to scroll horizontaly which is in general not convenient.

@slemeur
Copy link
Contributor

slemeur commented Jan 27, 2017

What is your proposal @gazarenkov ?

For the table's UI, we agree with your comment, with @akurinnoy we tried to display this table with one more column, but text is truncated and table looks worst than with 2 lines.
We thought about hiding the URL and just use a button, but we thought that it would not allow simple copy/paste action.

We do not really understand your latest comment, if you can explain clearly what you'd like to be displayed, it would be helpful and maybe solve properly the problem.

@ashumilova ashumilova removed this from the 5.2.0 milestone Jan 31, 2017
@benoitf
Copy link
Contributor

benoitf commented Mar 1, 2017

Can we merge this PR ? it is there since a long long time

@ashumilova
Copy link
Contributor

+1 on merging

@slemeur
Copy link
Contributor

slemeur commented Mar 7, 2017

Yes fine for me.

@ashumilova ashumilova added this to the 5.5.0 milestone Mar 7, 2017
Signed-off-by: Oleksii Kurinnyi <okurinnyi@codenvy.com>
@akurinnoy akurinnoy changed the title CHE-1579: show server's address along with URL CODENVY-1579: show server's address along with URL Mar 9, 2017
@codenvy-ci
Copy link

Build finished.
Build # 2142 - FAILED

Please check console output at $BUILD_URL to view the results.

@codenvy-ci
Copy link

Build # 2142 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/2142/ to view the results.

@akurinnoy akurinnoy merged commit 85d3578 into master Mar 10, 2017
@JamesDrummond JamesDrummond mentioned this pull request Mar 17, 2017
9 tasks
@cyberglad
Copy link

2017_05_12_09_54_22_codenvy_hello_world
I may sound stupid, but I don't see any icon at my upper-right corner, as I have seen on your screenshots. How do I switch to Service section?

@ashumilova
Copy link
Contributor

ashumilova commented May 12, 2017

@cyberglad by icon in the upper-right corner, i think, you mean machines/operation perspective switcher. Yes, we removed it. Now to see and modify servers - need to go to concrete workspace details in dashboard.
image
Select runtime tab and expand the details of necessary machine. There you will find Servers section.
Note, that workspace should be running.

@LiteSoul
Copy link

@ashumilova So you removed the Machine Perspective Switcher, so when you search on google trying to find Where to see the server ip one is running, the answers are wrong because there is no more icon.

Second, now we have to go to the Dashboard, then the workspace, then Runtime, then Dev-machine, then Server... whoa that's incredible hard. Knowing the ip is Fundamental and i'ts one of the most important tools we need.

I hope you can make it into a simple 1 click inside the ide... thanks

@ashumilova
Copy link
Contributor

@LiteSoul Thank you for feedbacks, we will take it into account to improve UX.

@akurinnoy akurinnoy deleted the CODENVY-1579 branch May 15, 2017 09:26
@cyberglad
Copy link

cyberglad commented May 22, 2017 via email

JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
Signed-off-by: Oleksii Kurinnyi <okurinnyi@codenvy.com>
skabashnyuk pushed a commit that referenced this pull request Jan 3, 2020
Signed-off-by: Oleksii Kurinnyi <okurinnyi@codenvy.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants