Open Sae126V opened 1 year ago
Looks good, I've left two comments/questions.
I spotted a couple of odd things:
Page_Type=User
changes based on whether "Last Logged In" is displayed or not. I presume this is because this line changes from being 1st to 2nd in the table. Can you move the "Last Logged In" the bottom of the "User Details" table?As part of this ticket, I have used the name like site_table_row_even and site_table_row_odd for the Last Logged In field instead of site_table_row_1 and site_table_row_2.
Please put this into it's own PR.
Can you also split out the broader line length and refactoring into seperate PRs as well?
with #477 being merged, is the PR still needed?
with #477 being merged, is the PR still needed?
Regarding the functionality wise. We can close this, Greg. I will create another PR for site_table_row_even
and site_table_row_odd
change if we need. Does that make sense ?
with #477 being merged, is the PR still needed?
Regarding the functionality wise. We can close this, Greg. I will create another PR for
site_table_row_even
andsite_table_row_odd
change if we need. Does that make sense ?
Can this be closed then? You can stick a speculative Issue in for the row changes even if you're not sure (there's a "question" tag for a reason), but it's better to tidy up unneeded PRs
To help with user support when someone queries one of the account deletion emails, it would be useful to be able to see when the account was last logged in to in the web interface.
As part of this ticket, I have used the name like
site_table_row_even
andsite_table_row_odd
for theLast Logged In
field instead ofsite_table_row_1
andsite_table_row_2
."Resolves #456 ".