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

Changed the behaviour of Prefix table: allow sorting #5452

Merged
merged 9 commits into from Mar 21, 2024

Conversation

timizuoebideri1
Copy link
Contributor

@timizuoebideri1 timizuoebideri1 commented Mar 19, 2024

Closes #4811

What's Changed

Screenshots

5d39e694-6b1b-46a8-a789-507242cf6a08.mp4

TODO

  • Explanation of Change(s)
  • Added change log fragment(s) (for more information see the documentation)
  • Attached Screenshots, Payload Example
  • Unit, Integration Tests
  • Documentation Updates (when adding/changing features)
  • Example Plugin Updates (when adding/changing features)
  • Outline Remaining Work, Constraints from Design

@itdependsnetworks
Copy link
Contributor

Are we doing this for all tree models (e.g. Tenant/Rack Groups) or only some?

@glennmatthews
Copy link
Contributor

Are we doing this for all tree models (e.g. Tenant/Rack Groups) or only some?

The actual TreeModel classes were previously addressed in #5218 and #5242.

changes/4811.changed Outdated Show resolved Hide resolved
Copy link
Contributor

@HanlinMiao HanlinMiao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have some screenshots/video demo in the PR description for ease of reviews?

@@ -42,28 +42,10 @@
{% if record.present_in_database %}{% utilization_graph record.get_utilization %}{% else %}—{% endif %}
"""

PREFIX_LINK = """
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this removed because it was unused? My only hesitation is whether we might have any apps that are relying on it that we don't know about.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not exposed to the apps/plugin, or do plugin developers use constants not exposed to apps?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They aren't supposed to but that doesn't mean they can't. Just being overly cautious here.

@glennmatthews glennmatthews merged commit f038c8f into next Mar 21, 2024
17 checks passed
@glennmatthews glennmatthews deleted the u/timizuoebideri-4811-prefix-ui-sorting branch March 21, 2024 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants