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

Improve loading page of Mac App #12327

Closed
gvol opened this issue Jan 19, 2012 · 18 comments
Closed

Improve loading page of Mac App #12327

gvol opened this issue Jan 19, 2012 · 18 comments

Comments

@gvol
Copy link
Contributor

gvol commented Jan 19, 2012

The current loading page of the Mac App is hideous and not as informative as it could be. Also the link to localhost:8000 will soon become localhost:8080 with the new flask notebook (see #11080). Both of these should be fixed without remorse.

Depends on #11080

CC: @kcrisman

Component: interfaces

Keywords: mac app, mac

Author: Ivan Andrus

Reviewer: Karl-Dieter Crisman

Merged: sage-5.2.beta0

Issue created by migration from https://trac.sagemath.org/ticket/12327

@gvol gvol added this to the sage-5.0 milestone Jan 19, 2012
@gvol gvol self-assigned this Jan 19, 2012
@kcrisman
Copy link
Member

comment:1

Yes! And tested without remorse!

@gvol
Copy link
Contributor Author

gvol commented Jan 19, 2012

comment:2

Replying to @kcrisman:

Yes! And tested without remorse!

Let it be so.

@kcrisman
Copy link
Member

comment:3

See here for the coffee quote.

@kcrisman
Copy link
Member

comment:4

More or less all looks ok. In line 127 we have

<p> If the server is already runn

but I think HTML will ignore the extra whitespace. I assume the CSS is all pretty nice; is there a reason not to use default sizes for things like h1, h2, h3?

It's taking too long for the dmg to build, so I'll have to finish the review tomorrow morning. Also modulo whether port 8080 actually is the new flask port, but #12310 and other references would seem to cover this if I make the new notebook a dependency. That makes this and #11080 dependencies of each other, but I think that's appropriate.

Unless... what about [#11080 comment:84]? That's not addressed here, is it addressed elsewhere? Otherwise you might want to make this still port 8000 so it can get merged and address that where you take care of the other issue.

@kcrisman
Copy link
Member

Dependencies: #11080

@kcrisman
Copy link
Member

Reviewer: Karl-Dieter Crisman

@kcrisman
Copy link
Member

Author: Ivan Andrus

@gvol
Copy link
Contributor Author

gvol commented Jan 27, 2012

comment:5

Replying to @kcrisman:

More or less all looks ok. In line 127 we have

<p> If the server is already runn

but I think HTML will ignore the extra whitespace. I assume the CSS is all pretty nice; is there a reason not to use default sizes for things like h1, h2, h3?

Yes it should ignore the whitespace. As to sizes of h1, etc. I just modified some css from sagemath.org so that I would get the colors right and they had changed the sizes.

It's taking too long for the dmg to build, so I'll have to finish the review tomorrow morning. Also modulo whether port 8080 actually is the new flask port, but #12310 and other references would seem to cover this if I make the new notebook a dependency. That makes this and #11080 dependencies of each other, but I think that's appropriate.

I think that's probably right.

Unless... what about [#11080 comment:84]? That's not addressed here, is it addressed elsewhere? Otherwise you might want to make this still port 8000 so it can get merged and address that where you take care of the other issue.

That's not addressed here, nor elsewhere. When I finally got 5.0.beta1 to build and tested it, it wasn't a problem. I'm going to comment on it over there as well now that you reminded me.

Now that I think about it people might run older versions of Sage and then it would be port 8000 anyway. It would be easy to put 2 links. I'm not sure it's worth it though since they should never have to click the link, and the server be running on a different port anyway since 8000 or 8080 might have been taken by something else (e.g. hg serve). It's trivial to add it, but I don't want to do it if it means you will have to build the dmg again to review it. FWIW you can set SAGE_APP_DMG to no to avoid building the dmg, though it still takes forever to build the application itself.

@kcrisman
Copy link
Member

comment:6

I'm not worried about the port; anyone who knows that much and is intentionally using an older (?!) version of Sage is going to know to use another port.

Ok, then. It looks great! The tips are amusing, though hopefully people won't have to go through all of them, and they don't seem to appear completely randomly.

As to building, my understanding is that SAGE_APP_DMG=no still builds Sage as a tgz, just not a dmg. Anyway.

@jasongrout
Copy link
Member

comment:7

I guess this line has a typo: "Sometimes that best way to solve find a bug is to explain it to a rubber duck "

@gvol
Copy link
Contributor Author

gvol commented Feb 5, 2012

Attachment: trac_12327-loading-page.patch.gz

Fixed typo

@gvol
Copy link
Contributor Author

gvol commented Feb 5, 2012

comment:8

Good catch, thanks.

@jdemeyer jdemeyer removed this from the sage-5.0 milestone Feb 14, 2012
@kcrisman
Copy link
Member

comment:10

Replying to @jdemeyer:

milestone changed from sage-5.0 to sage-pending

Is this because it's waiting on #11080? Just clarifying - that would certainly make sense.

@jdemeyer
Copy link
Contributor

comment:11

Replying to @kcrisman:

Is this because it's waiting on #11080?

and #11874 and #11503.

@jdemeyer jdemeyer added this to the sage-5.1 milestone Apr 8, 2012
@jdemeyer jdemeyer removed the pending label Apr 8, 2012
@jdemeyer jdemeyer removed this from the sage-5.1 milestone May 23, 2012
@jdemeyer jdemeyer added this to the sage-5.2 milestone Jun 2, 2012
@jdemeyer jdemeyer removed the pending label Jun 2, 2012
@jdemeyer
Copy link
Contributor

jdemeyer commented Jul 2, 2012

Merged: sage-5.2.beta0

@jdemeyer
Copy link
Contributor

comment:16

Unmerging this from sage-5.2 due to the serious security issue at #13270.

@jdemeyer
Copy link
Contributor

Changed merged from sage-5.2.beta0 to none

@jdemeyer jdemeyer reopened this Jul 23, 2012
@jdemeyer jdemeyer removed this from the sage-5.2 milestone Jul 23, 2012
@jdemeyer
Copy link
Contributor

Merged: sage-5.2.beta0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants