search

D61-IA / stellar-gnosis

Gnosis paper management and collaboration tool
Apache License 2.0
0 stars 1 forks source link

Feature/ego resize #68

Closed Zhenghao-Zhao closed 4 years ago

Zhenghao-Zhao commented 4 years ago

Features:

PantelisElinas commented 4 years ago

@Zhenghao-Zhao

can you please fix the conflict in the css file before I review this?

Thank you!

Zhenghao-Zhao commented 4 years ago

Fix applied. Thanks!

PantelisElinas commented 4 years ago

Hi @Zhenghao-Zhao

you have changed the layout and this is a rather major design change. Although the buttons all render correctly on mobile, I don't know if I like the new layout. I've been thinking that the ego-graph card need to be simplified. Let me have a think about it and we can discuss further on Thursday.

In the meantime, I noticed that on mobile, e.g. iPhone 5/SE, the displayed graph does not fit into the viewport and I cannot zoom out further to bring the whole graph into view. See the below image,

Screen Shot 2020-01-07 at 8 52 09 am

Thank you!

Zhenghao-Zhao commented 4 years ago

Fix applied for ego graph size issue, I am not sure if the dropdown menu is also an issue since you did not mention it.

Here are some benefits of this design I'd like to mention: it simplifies the front end code by removing resize code at the css and there was no JS code used; it also makes the canvas much clearer by not having buttons over it when on smaller sized screens. This is more valuable for screens that are even smaller such as iPhone 5.

Feel free to express your concerns here before Thursday so I'll prepare solutions/ideas for them.

Zhenghao-Zhao commented 4 years ago

Knowledge graph updated. Ready for review. Thanks!

PantelisElinas commented 4 years ago

Hi @Zhenghao-Zhao

I just had a brief look and in mobile view, clicking on an author node, doesn't take me to the author's detail view like in the desktop view. Can you have a look at this?

Thank you!

Zhenghao-Zhao commented 4 years ago

I find the issue has been there since the TechLauncher project. I am still trying to find a solution, by trying out different things. I will keep you updated here. Thanks!

Zhenghao-Zhao commented 4 years ago

Turns out there is a separate event name for touch devices and mouse input devices. The issue has now been fixed.

PantelisElinas commented 4 years ago

Hi @Zhenghao-Zhao

this looks good except for the two blue buttons, reset and T, where the icons are not centred in the button. Is this something you can fix before I merge this?

Are you also tackling #79 with this? Because it looks like it. If so, I wanted the endorse and bookmark buttons on their own row on mobile since on desktop there is plenty of space. If it is too difficult then we can have them on separate rows for both mobile and desktop.

P.

Zhenghao-Zhao commented 4 years ago

All selenium tests pass after the update.

The update does not have this, but what do you think if we reverse the positions of the title and the icon in desktop?

Annotation 2020-02-03 092204

PantelisElinas commented 4 years ago

Hi @Zhenghao-Zhao

The order of the icons and the title is best as it is now.

Cheers!

PantelisElinas commented 4 years ago

It looks good. I'll merge it!