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

"Mark planned" doesn't immediately change row highlight colour #13712

Closed
pv2b opened this issue Sep 7, 2023 · 3 comments · Fixed by #13874
Closed

"Mark planned" doesn't immediately change row highlight colour #13712

pv2b opened this issue Sep 7, 2023 · 3 comments · Fixed by #13874
Assignees
Labels
severity: low Does not significantly disrupt application functionality, or a workaround is available status: accepted This issue has been accepted for implementation topic: UI/UX User interface or user experience related work type: bug A confirmed report of unexpected behavior in the application

Comments

@pv2b
Copy link
Contributor

pv2b commented Sep 7, 2023

NetBox version

v3.6.1

Python version

3.11

Steps to Reproduce

Prerequisites: You need a device with a cable attached to one of its interfaces. The interface should not be disabled.

Since it's a client-side issue it's not impossible that the problem is browser specific. In case it turns out to be browser-specific, I've observed this behaviour on Firefox 117.0.

  1. Open the "interfaces" view of the device in question.
  2. Locate an interface with a cable connected to it. Note that the background colour of the row in the table is green to indicate it has a connected cable.
  3. Click on the "marked planned" button (the third button from the right)
  4. Notice that the colour of the button changed from yellow to blue to indicate that the cable is now planned, and no longer installed. However notice that the background colour of the row is still green.
  5. Refresh the page, and notice that the background colour of the row is now correctly blue.

Expected Behavior

The background colour of the row should have changed from blue to green as soon as the "Mark Planned" button was clicked.

Observed Behavior

The background colour of the row did not change, and a page refresh was required to correctly display the current state.

(P.S. I'm willing to troubleshoot this issue and produce a PR, but as per project rules, I cannot do this before the issue has been accepted and assigned to me.)

@pv2b pv2b added the type: bug A confirmed report of unexpected behavior in the application label Sep 7, 2023
@pv2b
Copy link
Contributor Author

pv2b commented Sep 7, 2023

To exclude this being browser-specific, I've also tested this on Microsoft Edge Version 116.0.1938.69. The exact same behaviour appears there, so this is not browser-specific.

@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation severity: low Does not significantly disrupt application functionality, or a workaround is available labels Sep 11, 2023
@pv2b
Copy link
Contributor Author

pv2b commented Sep 17, 2023

I took a look at this, I believe the problem is here:

row.classList.remove('success');
row.classList.add('info');

and here:

row.classList.remove('info');
row.classList.add('success');

This code seems to be intended to replace the info class with success on the row and vide versa, but probably somewhere along the line during the development process of Netbox, the classes for the row changes from info and success to green and blue.

The "quick fix" would probably be to just change these colours to the proper ones, but that would likely be incomplete because different row colours can happen in different scenarios, and anyway, you'd end up with some fairly complicated logic for class selection, so in order to fix this properly some refactoring would be in order.

One possible approach would be to factor any logic for styling of rows out of the Jinja2 templates. Instead, the templates would only apply styles as "cable-connected" or "cable-planned" or "interface-disabled" etc to the rows, and then use CSS to handle the presentation details. All the connectionToggle javascript toggle would then have to do would be to remove/add the cable-planned class to the row. The style of the button itself could also probably inherit somehow from the style of the parent row.

Anyway, still willing to take a stab at this, but my favored approach would probably involve some significant refactoring of the styling of table rows, and would probably have to involve cable connections not just on interfaces, but other pages as well.

@abhi1693 abhi1693 added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Sep 17, 2023
pv2b added a commit to pv2b/netbox that referenced this issue Sep 18, 2023
Fixes netbox-community#13712 and netbox-community#13806.

Not super happy with the fix here, because it doesn't address the
underlying problem, which is that the toggleConnection() typescript function hardcodes
which CSS classes should be added/removed. Probably a more permanent fix would be
to stop applying CSS classes on the table view, and instead apply attributes
for cable/interface state, and then use CSS to apply colours based on
interface state, but this is a quite involved process. But it does at least
fix things in the here and now.
@DanSheps DanSheps added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: accepted This issue has been accepted for implementation labels Jan 23, 2024
@DanSheps
Copy link
Member

Since the previous issue was closed due to a lack of activity, I am putting this back to needs owner for now. If someone would like to take this on, just reply here.

@DanSheps DanSheps added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Jan 23, 2024
@DanSheps DanSheps added the topic: UI/UX User interface or user experience related work label Jan 23, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
severity: low Does not significantly disrupt application functionality, or a workaround is available status: accepted This issue has been accepted for implementation topic: UI/UX User interface or user experience related work type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
4 participants