iVis-at-Bilkent / newt

A web application to visualize and edit pathway models
http://newteditor.org
GNU Lesser General Public License v3.0
53 stars 27 forks source link

Make map type more apperant #715

Closed ugurdogrusoz closed 3 months ago

ugurdogrusoz commented 3 months ago

Currently, we have maps of these types: SBGN: PD, AF, PD+AF, SIF, SBML, and All. One can see the map type under Map Properties: image

In order to make this more apparent, let's also display it somewhere like here: image

Notice that when the user changes the map type, this should be properly updated.

umut-er commented 3 months ago

The baseline functionality for this has been done. It currently looks like this:

Screenshot 2024-06-25 at 16 52 58

let me know if something is wrong or if any changes are needed. There are two issues that I am aware of (that I will work on): 1 - When loading samples, we only get PD as the type. 2 - When changing map type with an undo operation, we can't change this indicator.

PS. The internal server appears to be down. I haven't uploaded these changes there yet.

umut-er commented 3 months ago

Other potential concerns include: 1 - remote launching 2 - importing if you have additional scenarios in which this feature could potentially fail (currently it works when new empty map is created and when the map type is changed using the sidebar (map -> general -> type)), please add them below.

ugurdogrusoz commented 3 months ago

Looks good. Can we make the style of this label different by reversing the colors? Background black, font color white for instance.

umut-er commented 3 months ago

I think this should be mostly done. The changes are now merged. Please review.

ugurdogrusoz commented 3 months ago

Looks great. Let's also make sure:

PD+AF PD+AF+SIF+SBML (ALL in tab labels due to space constraint)

umut-er commented 3 months ago

For the second part:

I wondered about the weird border as well. I don't think my changes are directly related to that weird corner shape but I will take a look at it.

For the first part:

I am confused about this image. The actual active map (which is currently of type ALL) is not one of those that are shown in the image (bottom left), right? Because that would imply a type inconsistency between the map type and the tab label.

From what I can understand we want to change this "Hybrid(PD,AF...)" text on the image to "PD+AF+SIF+SBML" (and the same for PD+AF). Is that accurate?

umut-er commented 3 months ago

The tabs currently look like this on my local:

image

I added box shadows and removed the left border. The following are the new error messages:

image image

Here is a overview of what has been done to resolve this issue:

  1. A new html element (a div) is added to display the map type. The positioning is handled similarly to the close tab symbol.
  2. Importing, opening a sample and remote launching are handled from a single place. These trigger either "sbgnvizLoadFileEnd" or "sbgnvizLoadSampleEnd" events. We just change the tab label when we encounter these events.
  3. Changing the map type from the sidebar is handled seperately as it works somewhat differently from the others in the background (because it doesn't create a new map and it is undoable).
  4. DETAIL ALERT: For requested change number 2 (number 1 is trivial), I found that weird border thing happened because of a -1px bottom margin on the parent \<li> item. This caused the last 1px of the border to not properly render. I however couldn't remove this -1px margin as it came from a library (I think) and not from our code. Instead, I matched the border radius of the child element to that of the parent element. This causes the issue to go away because there is no disagreement between the elements and hence no weird rendering. I also removed the left border and added some box shadowing which I think looks cool.

Please review this issue and close it if it is appropriate.

ugurdogrusoz commented 3 months ago

This looks great! The only last improvement I can suggest is making the shadow color of non-current tabs lighter.

umut-er commented 3 months ago

Here is a preview:

https://github.com/iVis-at-Bilkent/newt/assets/102927626/19ba1d6e-8a45-4e43-9f1d-fc2b1a39a714

Is this OK or should there be more contrast?

PS. The border feature is currently deployed (without this addition) on the internal server.

ugurdogrusoz commented 3 months ago

More contrast please; specifically non-active shadows should be lighter or narrower

ugurdogrusoz commented 3 months ago

Did we accidentally modify the style of the tabs on the right side (should be gray background, dark gray/black font and more compact - less padding)? Now:

Screenshot 2024-06-26 at 17 19 33

Was:

Screenshot 2024-06-26 at 17 20 01

Let's change it back making the style of the two as similar as possible.

umut-er commented 3 months ago

Yes, this issue was indeed introduced by me in commit 5a2e78cd2fc2cc2052cd88d0af1cb80ea1579016 (which is the first commit I made about this issue). It was a minor oversight on my part and resolved by putting some code I deleted in the aforementioned commit. I made sure that this error was contained to the elements you pointed out and not present anywhere else after I fixed it.

ugurdogrusoz commented 3 months ago

Now it's back to what it was but not consistent with document tabs, can we make these right panel tabs to have a shadow as well similarly?

ugurdogrusoz commented 3 months ago

Can we make the shadow of the active right panel tab darker, similar to the active document tab?

hasanbalci commented 3 months ago

I get error in the console when I try to change map type from map properties (map -> general -> type).

umut-er commented 3 months ago

Apparently one of Noor's commits (c99b32d0d900cf684c3cbe0cb899085ea48b8937) overwrote one of my changes accidentally. I changed it back. Please see it yourself on the internal server.