3drepo / 3drepo.io

3D Repo web server
http://www.3drepo.io
GNU Affero General Public License v3.0
95 stars 38 forks source link

4.5.1 bug list #1966

Closed carmenfan closed 4 years ago

carmenfan commented 4 years ago

Bugs

carmenfan commented 4 years ago

Background colour for comments is inconsistent This area should be grey: image There is a white strip if there is a comment: image This area should be grey: image

@m1chalkubiak can you take a look at this? I think this is caused by this fix: https://github.com/3drepo/3drepo.io/issues/1938#issuecomment-601683334

m1chalkubiak commented 4 years ago

Hi @carmenfan

@m1chalkubiak can you take a look at this? I think this is caused by this fix: #1938 (comment)

You are right I'm trying to figure out it right now.

carmenfan commented 4 years ago

Pins missing if you go into the model after looking at kanban

carmenfan commented 4 years ago

Lines should only appear below the name if it's in editing mode

image

carmenfan commented 4 years ago

The save/cancel button should appear as soon as it enters edit mode They currently only appears when the text has changed 1

carmenfan commented 4 years ago

CoordView is showing even though it's suppose to be disabled It needs to be turned off within unity and rebuild (CF: I'll do this)

anorrie commented 4 years ago

Existing text not clearing in edit mode When editing measurement name the existing text should be automatically cleared when entering edit mode

anorrie commented 4 years ago

Measure colour selector missing previously used options. In measure card the colour selector should have previously used options like in the colour selector for Jobs. This only needs to be remembered in the session.

carmenfan commented 4 years ago

X Y Z should align the X Y Z labels should align, but the numbers should stay where they are. (i.e. the units should also align. In other words you can think of this as 2 columns, the labels aligns vertically, and the data aligns to the right. image

m1chalkubiak commented 4 years ago

Hi @carmenfan

Lines should only appear below the name if it's in editing mode

Just to be clear we changed this behavior in purpose in ISSUE 1847 - SafetiBase v3 https://github.com/3drepo/3drepo.io/issues/1847#issuecomment-581864641 And in my comment above the linked one, I was asking should we change it to all places where this component is used. I understand, that now we would like this component to display differently in some places?

The save/cancel button should appear as soon as it enters edit mode They currently only appears when the text has changed

Existing text not clearing in edit mode When editing measurement name the existing text should be automatically cleared when entering edit mode

For both of the above, I would like to ask If this change should be made all around application, so in all places where this component is used?

carmenfan commented 4 years ago

Just to be clear we changed this behavior in purpose in ISSUE 1847 - SafetiBase v3

1847 (comment)

And in my comment above the linked one, I was asking should we change it to all places where this component is used. I understand, that now we would like this component to display differently in some places?

Ah yes sorry, I didn't realise we'd reuse this component for the measuring labels, Measuring labels shouldn't have a line below it, but issue/risks properties should. I hope this makes sense!

For both of the above, I would like to ask If this change should be made all around application, so in all places where this component is used?

Yes (as far as I'm aware, if there's a case where it seems strange to do, please let me know and we should review). But instead of clearing the text, it would be ideal if we can set the existing name as default text, so it still displays but you just right over it, like we have with the description field with the (no description) here: 1

m1chalkubiak commented 4 years ago

Just to be clear we changed this behavior in purpose in ISSUE 1847 - SafetiBase v3

1847 (comment)

And in my comment above the linked one, I was asking should we change it to all places where this component is used. I understand, that now we would like this component to display differently in some places?

Ah yes sorry, I didn't realise we'd reuse this component for the measuring labels, Measuring labels shouldn't have a line below it, but issue/risks properties should. I hope this makes sense!

No worries, from what I have checked this component is also used in edit view on group panel, should it have a line there or not?

For both of the above, I would like to ask If this change should be made all around application, so in all places where this component is used?

Yes (as far as I'm aware, if there's a case where it seems strange to do, please let me know and we should review). But instead of clearing the text, it would be ideal if we can set the existing name as default text, so it still displays but you just right over it, like we have with the description field with the (no description) here: 1

I have big doubts about this change. The main threat I see is the fact that we will not be able to edit previously entered text. It seems to me that this is not experience witch users would expect. Mostly I'm thinking about long descriptions in risk panel. However, for measurements panel where we have some "random" names, it makes sense in my opinion.

carmenfan commented 4 years ago

No worries, from what I have checked this component is also used in edit view on group panel, should it have a line there or not?

If you mean the description part yes, that should have a line underneath

I have big doubts about this change. The main threat I see is the fact that we will not be able to edit previously entered text. It seems to me that this is not experience witch users would expect. Mostly I'm thinking about long descriptions in risk panel. However, for measurements panel where we have some "random" names, it makes sense in my opinion.

@m1chalkubiak Alternatively, can we start the editing mode with the text highlighted? then the user can just type to overwrite it, or click where appropriate to edit?

m1chalkubiak commented 4 years ago

@carmenfan

If you mean the description part yes, that should have a line underneath

Yes, its used in for description field.

@m1chalkubiak Alternatively, can we start the editing mode with the text highlighted? then the user can just type to overwrite it, or click where appropriate to edit?

Yes, in my opinion, it sounds like a good idea. I will add this behavior, then we could check if it works as expected.

anorrie commented 4 years ago

Polygon area tool not clearing If you start an area measure and do not commit, after leaving the tool and re-entering the old unfinished shape will appear. Steps to recreate. 1) Start polygon area tool 2) click a number of points 3) DO NOT double click to commit 4) exit the polygon area tool 5) shape will disappear 6) re-enter tool 7) start drawing 8) old unfinished shape will appear

m1chalkubiak commented 4 years ago

Polygon Area Tool Not Clearing If you start an area measure and do not commit, after leaving the tool and re-entering the old unfinished shape will appear.

@carmenfan I think that we would need to have some additional method to tell Unity (from frontend) that we cancel creating of the area measurement

carmenfan commented 4 years ago

Polygon Area Tool Not Clearing If you start an area measure and do not commit, after leaving the tool and re-entering the old unfinished shape will appear.

@carmenfan I think that we would need to have some additional method to tell Unity (from frontend) that we cancel creating of the area measurement

don't worry about that one, i'll just fix it in unity. it should clear the cache when the tool changes/turned off

anorrie commented 4 years ago

Measurements don't always match Occasionally, measurement readings on the label and in the card do not match...

measure

carmenfan commented 4 years ago

Measurements don't always match Occasionally, measurement readings on the label and in the card do not match...

measure

@m1chalkubiak i think this is a rounding method difference, I will fix this as well in Unity (so it sends rounded numbers back to the front end)

m1chalkubiak commented 4 years ago

@m1chalkubiak i think this is a rounding method difference, I will fix this as well in Unity (so it sends rounded numbers back to the front end)

@carmenfan I'm not sure if this will cover all the cases, rounding is also done when user change the units from panel menu, then we don't get any update from Unity, so on the frontend I need to re-calcualate values manually.

Therefore, I believe we should be consistent on both sides with rounding numbers in the same way. However, if it's only happening in the case of "m" units, I probably know what can cause wrong rounding.

@anorrie I would be grateful if You could repeat this measurement with the newest version of the code, as I wasn't ably to find myself a case were rounding was done incorrectly.

m1chalkubiak commented 4 years ago

@m1chalkubiak Alternatively, can we start the editing mode with the text highlighted? then the user can just type to overwrite it, or click where appropriate to edit?

@carmenfan I just pushed change of this behavior, so now in all places where we are using this component, it will be the same. Just for reminding those places are: Risk/Issues descriptions, Group descriptions and measurements names.

carmenfan commented 4 years ago

Therefore, I believe we should be consistent on both sides with rounding numbers in the same way. However, if it's only happening in the case of "m" units, I probably know what can cause wrong rounding.

You're right. But we'll still tweak the numbers in unity first to the nearest mm, as that's the level of accuracy we support. so rounding should never be a problem for mm units

carmenfan commented 4 years ago

Therefore, I believe we should be consistent on both sides with rounding numbers in the same way. However, if it's only happening in the case of "m" units, I probably know what can cause wrong rounding.

You're right. But we'll still tweak the numbers in unity first to the nearest mm, as that's the level of accuracy we support. so rounding should never be a problem for mm units

@m1chalkubiak just had a look, seems like it's already is doing this by default. So no changes needed on the unity side

carmenfan commented 4 years ago

We can no longer save views with the default name

1

m1chalkubiak commented 4 years ago

@carmenfan I manage to solve issue with missing pins after visiting kanban board which took me most of the day yesterday to debug this.

I was also re-thinking case with saving default values and as a very quick fix I added condition that if the value didn't change we just exiting the edit mode (exactly like pressing the cancel button). Because, if we haven't made any changes, saving the same value again probably isn't needed at all. I hope this makes sense.

BDavydov commented 4 years ago

Limit the measurement name to about 15 characters per line

When you create a name and the measurement distance is long, both can get too close together. Can we drop to the next line after let's say 15 characters? This way we would avoid the name getting too close in line with the measurement.

image

BDavydov commented 4 years ago

Could we ditch the current Total Length arrangement?

Shift 'Selected total' to the right and make use of the white space to the right of the measurement.

Measurement Length Total Total Length Total Length collapsed

carmenfan commented 4 years ago

I was also re-thinking case with saving default values and as a very quick fix I added condition that if the value didn't change we just exiting the edit mode (exactly like pressing the cancel button). Because, if we haven't made any changes, saving the same value again probably isn't needed at all. I hope this makes sense.

@m1chalkubiak if you're talking about the viewpoint saving bug mentioned here then it won't work, as it hasn't been saved yet.

BDavydov commented 4 years ago

Save the name when you hit Enter button

Currently, you have to click on the save icon in order to save the new name of any measurement. Can we do it similarly to views, where you can write the name and hit Enter to submit it? image

BDavydov commented 4 years ago

Description 2 things:

Steps to replicate

EDIT by CF: The edit buttons shouldn't show up on new issues/risks.

image image

BDavydov commented 4 years ago

Edit by CF: This is a repeat of https://github.com/3drepo/3drepo.io/issues/1966#issuecomment-613325683

**Description** Similarly to SafetiBase when I create a new Issue I have to manually delete the Untitled Issue and Description contains 'Clear' & 'Delete' options which are inactive. Create a new issue and open it again to edit the Description field. Clear all button does not work. **Steps to Replicate:** - Open any model - Open the Issue Card and create a new Issue - Manually delete the Issue title in order to write a new one - Description contains options icons but they do not respond on click - Open the newly created issue to edit the description field - Click on the description edit button - Description clear all button X does not work ![image](https://user-images.githubusercontent.com/19904513/79208799-74bd7e00-7e3a-11ea-8303-6491f604caa1.png)
BDavydov commented 4 years ago

Can we show XYZ for measurements as per settings? Currently, it is showing X Z Y

image image

EDIT by CF: I'll fix this

m1chalkubiak commented 4 years ago

@m1chalkubiak if you're talking about the viewpoint saving bug mentioned here then it won't work, as it hasn't been saved yet.

Sorry @carmenfan mistakenly assumed that they were using the same textfield component that was changed, but it appears that its not the case. So, I wrote additional logic that sends default name if none were added during creating of new viewpoint. However, I wasn't able to find a version of an application on which it would work differently. I back to version 4.4.1 from over a month ago and it was acting the same.

carmenfan commented 4 years ago

@m1chalkubiak if you're talking about the viewpoint saving bug mentioned here then it won't work, as it hasn't been saved yet.

Sorry @carmenfan mistakenly assumed that they were using the same textfield component that was changed, but it appears that its not the case. So, I wrote additional logic that sends default name if none were added during creating of new viewpoint. However, I wasn't able to find a version of an application on which it would work differently. I back to version 4.4.1 from over a month ago and it was acting the same.

Yes I happened to test that since I was looking around simlar looking components to see if they work. Not sure when it started breaking!

Just tried, it working well, thanks!

I noticed you checked off https://github.com/3drepo/3drepo.io/issues/1966#issuecomment-613325683 but it doesn't automatically highlight the title when you go into new issue/risk. Can we change that as well? image

m1chalkubiak commented 4 years ago

I noticed you checked off #1966 (comment) but it doesn't automatically highlight the title when you go into new issue/risk. Can we change that as well?

Ohh I see, it was about the second part - Edit buttons shouldn't appear when the user is creating a new issue/risk I will check right away if can we automatically highlight the default title, as You suggested.

carmenfan commented 4 years ago

Change teamspace setting update to a patch instead of put

At the moment we expect a put http request for teamspace setting update when it should be a patch

(I'll work on this)

carmenfan commented 4 years ago

We're still using the RISK_CATEOGORIES constant in places

for e.g. Kanban board filter image

RISK_CATEOGORIES should be completely gone and replaced with the dynamic one from API

carmenfan commented 4 years ago

@m1chalkubiak we still have a selectTopicType in the model selector and it seems like the kanban board is using this. This should be wired to the selector from teamspace right? We need to remove that one and make sure everything comes from the teamspace selector.

I'm also seeing a mismatch between the topic type on kanban filter vs the topic types in teamspace setting. (looks like it's a default list)

m1chalkubiak commented 4 years ago

@m1chalkubiak we still have a selectTopicType in the model selector and it seems like the kanban board is using this. This should be wired to the selector from teamspace right? We need to remove that one and make sure everything comes from the teamspace selector.

I'm also seeing a mismatch between the topic type on kanban filter vs the topic types in teamspace setting. (looks like it's a default list)

@carmenfan You're right, the board has been omitted from this refactor. So from tomorrow morning, I will start looking at what exactly needs to be changed there to work with dynamic data from API.

BDavydov commented 4 years ago

Description Custom area measurement gives an error message when surface is large

Steps to replicate:

image

BDavydov commented 4 years ago

EDIT by CF: Sebastian is looking at this.

Description Selected objects get a ghost effect when rotating or panning around the model. This is not the issue for the same model on Prod.

Steps to Replicate:

Currently on DEV: Object Ghost Effect

Currently on PROD: Object No Ghost Effect

BDavydov commented 4 years ago

Description Action Buttons don't work for any of the features in the model viewer (Issues, SafetiBase, Groups & Views) & Compare Card does not load

Steps to replicate:

Action Button not responding

When I try to create a new Issue: image

m1chalkubiak commented 4 years ago

Description Action Buttons don't work for any of the features in the model viewer (Issues, SafetiBase, Groups & Views) & Compare Card does not load

@BDavydov Quick guess, can You check what is visible in Teamspace Settings page of this model? Unfortunately, I don't have access to this model by my account.

carmenfan commented 4 years ago

Description Action Buttons don't work for any of the features in the model viewer (Issues, SafetiBase, Groups & Views) & Compare Card does not load

@BDavydov Quick guess, can You check what is visible in Teamspace Settings page of this model? Unfortunately, I don't have access to this model by my account.

@m1chalkubiak I have acces, I'll take a look first

carmenfan commented 4 years ago

Description Action Buttons don't work for any of the features in the model viewer (Issues, SafetiBase, Groups & Views) & Compare Card does not load

@BDavydov Quick guess, can You check what is visible in Teamspace Settings page of this model? Unfortunately, I don't have access to this model by my account.

@m1chalkubiak I have acces, I'll take a look first

@m1chalkubiak in Issues saga we're still using the model selector for topic types, as this is a new model the model API doesn't return the topic type list anymore (we haven't ran the script to remove the old ones yet, but the new ones doesn't generate anymore). So what you're doing right now should fix this. Though I don't think we can assume topic_type must have a length of at least 1, so i'm going to add in some checks.

Compare card problem is a separate problem, I haven't gotten to the bottom of it yet but it looks like it's a backend problem.

carmenfan commented 4 years ago

Updating risk category is updating everything else

  1. create a risk
  2. change the risk category from blank to something
  3. you can see the comment log is populated with a lot of actoins...

1 Looks like this is a back end bug as the front end is only sending 1 value

sanmont3drepo commented 4 years ago

Updating risk category is updating everything else

  1. create a risk
  2. change the risk category from blank to something
  3. you can see the comment log is populated with a lot of actoins...

1 Looks like this is a back end bug as the front end is only sending 1 value

done (it was a problem in the frontend when evaluating the values that changed in the form)

BDavydov commented 4 years ago

Description Measurement banners appear to be floating randomly in the model as you move to other model areas. There is no measurement taken, but the banner appears there.

Steps to replicate

Measurement Banner 1

Measurement Banner 2

BDavydov commented 4 years ago

Description Error message comes up when you are deleting the incomplete custom surface area measurement.

Steps to replicate:

image

BDavydov commented 4 years ago

Description

Steps to replicate

Point Measurement