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

Change Quota Status Display Wording #2810 #2818

Merged
merged 1 commit into from
Mar 26, 2024

Conversation

Hooverdan96
Copy link
Member

Fixes #2810.

update to shares_table.jst

Testing:
Changed wording (including Quotas to clarify what this status refers to) shows up on screen now:

image

For good measure, also ran test suite.

# poetry run django-admin test -v 2

----------------------------------------------------------------------
Ran 279 tests in 46.885s

OK

update shares_table.jst
@Hooverdan96 Hooverdan96 changed the title Change Quota Status Display Wording Change Quota Status Display Wording #2810 Mar 26, 2024
@FroggyFlox
Copy link
Member

Probably off topic here, but what do you guys think about the red font used here? I wonder if that could lead some to think it's an actual error more than a setting for the share/pool.

@Hooverdan96
Copy link
Member Author

@FroggyFlox you've got a point there. If quotas are eventually enforced, I would consider it a warning, since any share could consume the entire pool. In our current setup that risk is there with quotas on or off, since Rockstor does not enforce it (yet). But, in general, the warning/alert would be more relevant in the pools page, since that's where it's being set?

@phillxnet
Copy link
Member

Reviewing this now: as per colour change: lets address that in it's own issue as there are other locations affected by that. Here I think we just need for the warning (ergo red) to be more clear on what it is warning about. And this more explicit wording does just that I think/hope.

Copy link
Member

@phillxnet phillxnet left a comment

Choose a reason for hiding this comment

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

@Hooverdan96 Thanks for seeing to this change: much appreciated.

When I originally added this wording I had not appreciated it's potential to mislead via its brevity; and we now have longer cell entries in this column as a result: but better over all and with some improvements in our tables we can always reclaim some of the wasted space in many of the other columns.

With an rpm built including this fix we have the expected outcome of better wording, after having refreshed browser cache that is:

quotas-disabled-wording-change-pr

Cheers, I'll go ahead and get this merged ready for our next testing rpm.

@phillxnet phillxnet merged commit 792d5b7 into rockstor:testing Mar 26, 2024
@Hooverdan96
Copy link
Member Author

Just as a sidenote, if we choose to do away with the red color just in this instance (and/or the bolding), it's in the same line as what this PR's change did:

do away with the tag around the text:

<strong><span style="color:red">Quotas Disabled</span></strong>

@Hooverdan96 Hooverdan96 deleted the 2810_Quotas_Disabled_webui branch March 26, 2024 18:36
@phillxnet
Copy link
Member

@Hooverdan96
Re:

if we choose to do away with the red color just in this instance

In the context that we show folks data as 0 bytes when quotas are disabled: I'm very much inclined to still treat this as a 'red' condition. I now we have grown accustom to quotas disabled but I think we need to move towards actually enforcing them soon. And we do still have quotas enabled as a default. We have after all just seen on the forum a discussion where folks were to delete shares as no data, i.e. zero bytes, was occupied: ergo zero loss - or data already gone. This was the motivation for me creating the issue. To emphasis that it is only the quotas that was disabled.

We do have the mouse-over explaining the zero space reading with quotas disabled - but bit-by-bit. Maybe a parallel UI improvement here is to replace the zero (when quotas disabled) with something like "unknown" with mouse-over to indicate quotas disabled. But this is all for the next testing phase I think, as the hope is we can make such changes after updating/easing our Web-UI out from under near raw JS: and have more live elements.

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.

3 participants