nautobot / nautobot-app-floor-plan

Easily view floor plans in Nautobot
https://docs.nautobot.com/projects/floor-plan/en/latest/
Other
11 stars 5 forks source link

Add usage of letters for rendering FP SVG. #79

Closed pszulczewski closed 6 months ago

pszulczewski commented 7 months ago

This draft adds only SVG rendering options to use letters or numbers for the grid numbering axis labels. Selecting letters for the X axis will make the FP look like a chessboard. obraz

The downside is that for adding tiles it still uses numbers, because FloorPlanTile model x_origin and y_origin fields are now PositiveSmallIntegerField.

obraz

Potentially form could be changed to present numbers or letters for tile origins, and under clean() it could be covered back to numbers.

joewesch commented 7 months ago

Potentially form could be changed to present numbers or letters for tile origins, and under clean() it could be covered back to numbers.

I like this idea. Let's do it!

itdependsnetworks commented 7 months ago

I had mentioned that we should consider having a generator for the increment https://github.com/nautobot/nautobot-app-floor-plan/issues/8#issuecomment-1423580189

pszulczewski commented 7 months ago

The latest commit is to support letters in the tile form.

obraz

@itdependsnetworks I am fine with implementing generator if @joewesch allows more time.

joewesch commented 7 months ago

I had mentioned that we should consider having a generator for the increment

I like the idea, but I think what we have so far will be a good start as I believe it covers at least 80% of use cases. Let's count this as incremental progress toward the eventual goal and not mark this as closing/resolving the issue.

pszulczewski commented 7 months ago

I am still thinking if keeping x/y grid setting per object will give good user experience? Maybe it should be a global plugin setting, which is selected only once?

joewesch commented 7 months ago

I am still thinking if keeping x/y grid setting per object will give good user experience? Maybe it should be a global plugin setting, which is selected only once?

Yes, a plugin setting default would be great, but still allow for grid specific override. Our client uses different grid patterns per Location.

pszulczewski commented 7 months ago

What is the changelog in the CI? I am not sure why it fails?

joewesch commented 7 months ago

What is the changelog in the CI? I am not sure why it fails?

This repo uses towncrier and requires a changelog fragment file: https://docs.nautobot.com/projects/floor-plan/en/latest/dev/contributing/#creating-changelog-fragments

pszulczewski commented 7 months ago

What is the changelog in the CI? I am not sure why it fails?

This repo uses towncrier and requires a changelog fragment file: https://docs.nautobot.com/projects/floor-plan/en/latest/dev/contributing/#creating-changelog-fragments

Thanks. Looks like my branch was checked out beginning of February and it didn't have changes directory. I've just re-based on develop and added a change file.

joewesch commented 7 months ago

Patryk, something isn't working right with the edit form. If I set one (or both) of the labels to letters and then edit the floor plan, the form field defaults back to numbers.

joewesch commented 7 months ago

When setting a label to use letters, but then putting a number in the form it raise an inconsistent error message: Screenshot 2024-03-25 at 12 14 50 PM

As part of the clean method we should do one of the following:

pszulczewski commented 7 months ago

Let me check that.

pszulczewski commented 7 months ago

Patryk, something isn't working right with the edit form. If I set one (or both) of the labels to letters and then edit the floor plan, the form field defaults back to numbers.

Looks like it stopped working after changing form field to CharField. I fixed that and added validations. Should be good now, can you please pull and re-test?

pszulczewski commented 7 months ago

@joewesch I have added unittests for the FloorPlanTileForm because there weren't any for that form, and they revealed that there were still some issues. I've made the necessary updates in forms, we should be good now.

joewesch commented 7 months ago

@glennmatthews would you mind giving this a once over?