Submitty icon indicating copy to clipboard operation
Submitty copied to clipboard

Published 20 hours ago • verified publisher icon Submitty

[Bugfix:TAGrading] Links to buttons

Open jeninyg opened this issue 2 years ago • 6 comments

What is the current behavior?

Help With #7819

jeninyg avatar Apr 29 '22 21:04 jeninyg

Codecov Report

Merging #7863 (b06a96f) into main (81f226e) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #7863   +/-   ##
=========================================
  Coverage     22.15%   22.15%           
  Complexity     7682     7682           
=========================================
  Files           205      205           
  Lines         24181    24181           
  Branches         70       70           
=========================================
  Hits           5358     5358           
  Misses        18755    18755           
  Partials         68       68
Flag Coverage Δ
autograder 19.22% <ø> (ø)
js 28.47% <ø> (ø)
migrator 99.11% <ø> (ø)
php 20.48% <ø> (ø)
python_submitty_utils 71.65% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

codecov[bot] avatar Apr 29 '22 21:04 codecov[bot]

You will need to update the selenium tests to use the new selector - but if you're interested it would be a good experience to move the test to cypress - since it'll probably be easier to develop/maintain in the future

shailpatels avatar Apr 29 '22 22:04 shailpatels

@jeninyg Can you please make the PR message more informative?
Please explain the purpose of this PR (copy paste from the issue if you like).
Putting a link to the original issue is not enough.

Also, does this close the issue? Does it fix all instances of this problem? Or just some of them?
(Github seems to be ready to close the issue -- but maybe that is stale from when you wrote "Fixes" and now that it says "helps with" it wont?). - @shailpatels please confirm

bmcutler avatar May 10 '22 01:05 bmcutler

@bmcutler This PR fixes a couple of instances but not all of them, this section from Mozilla is a good overview of why not to use links as buttons - the original issue also has some extra context

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#onclick_events

shailpatels avatar May 10 '22 01:05 shailpatels

@immortalcodes @shailpatels Is PR #7906 duplicating these changes?

Should we close this PR in favor of 7906? If we don't close it, @immortalcodes, please fix this PR message, etc as discussed above.

bmcutler avatar Jun 03 '22 19:06 bmcutler

The changes are different this one is only on user profile page, I still need to update the tests to pass with the changes. Was hoping to move them to cypress at the same time but the migration to 10x got in the way

shailpatels avatar Jun 03 '22 19:06 shailpatels