Change back button to last viewed page #1522

varCepheid · 2023-01-15T21:06:04Z

closes #1497
This PR takes away the back attribute in the Page element. It also adds a link to the event on event-specific pages and new pages for editing reports separate from the pages to view them.

calebeby · 2023-01-26T01:08:30Z

@varCepheid The way I designed the "back system" is that the back button goes "up a level", when that makes sense, and when there is no logical "up a level", it will perform the same action as the browser back button (go to the previous page).

Sometimes, that system is confusing. Instead of changing the behavior on this one individual page, can you propose a new logical system for the back button across all pages?

calebeby · 2023-01-26T01:10:26Z

I acknowledge that the current system is confusing. Do you think it'd make sense for the back button in all cases to work the same as the browser back button? I would not be opposed to that kind of change.

However, if we did that, then there would be no easy way to, for example, navigate from a match page to the event corresponding to that match, unless that is the page you came from.

What do you think would make the most sense?

varCepheid · 2023-01-26T01:14:13Z

I think your fix makes more sense, and there should be a link to the event page on the match page.

moved the link under the name of the match and removed it from the event page

varCepheid · 2023-02-09T02:04:29Z

Note to self: finish working on match details card, seems to only be updating on event page; then fix the type errors.

varCepheid · 2023-02-10T05:08:56Z

@calebeby routes/report, line 51: The back button had stopped editing mode when the report was being edited without changing the page. Now, the back button will always go to the browser's back no matter what. Should there be a button on the report page to stop editing, or is the Save Report button at the bottom good enough?

calebeby

Hi @varCepheid! Thanks for your patience with me! I left some feedback. I got a chance to review almost all of the files in this PR, except I haven't had time to closely review the changes to the report editor yet (when big blocks of code are moved the diff view doesn't really show what, if anything was actually changed, so it takes longer to compare)

src/routes/event-match.tsx

src/components/match-card.tsx

src/components/page.tsx

src/routes/report.tsx

src/routes/saved-report-edit.tsx

src/routes/event-team.tsx

changed homepage argument of Page & Header to showBackButton and flipped the logic changed onEditClick argument of report-viewer to reportEditorLink and changed the Button to href shortened functions in saved-report-edit

…uidelines

varCepheid · 2023-11-07T23:07:53Z

Thanks for all the notes! I went with a text-decoration-color attribute for the last point and I think it looks good. I won't merge until you have a chance to look at the report-editor component.

varCepheid · 2024-02-05T01:46:56Z

@calebeby I've addressed all of the changes you've requested. Can you add another review or approve the changes?

src/colors.ts

src/components/page.tsx

calebeby

Great work here! I still have yet to review the report editor changes, I can't remember why those are in this PR and I don't have time to look closely right now.

src/routes/event-analysis.tsx

src/routes/event-match.tsx

src/routes/event-team-comments.tsx

src/routes/event-team-matches.tsx

src/routes/event-team.tsx

varCepheid · 2024-02-17T00:58:10Z

The changes to the report and saved-report routes are because the logic on the back button used to discard changes to a report being edited, but with this PR in order to maintain that logic we needed new pages.

We Do Not Need These Extra Spaces --Caleb Eby

calebeby

@varCepheid This is looking really good! I finally remembered to come back here to look at the report viewer/editor changes. I left some comments.
Also feel free to bug me on slack if I forget to review things again!

src/components/report-viewer.tsx

calebeby · 2024-07-05T17:41:36Z

src/routes/event.tsx

@@ -73,7 +69,6 @@ const Event = ({ eventKey }: Props) => {
        </Heading>
        {newestIncompleteMatch && (
          <MatchDetailsCard
-            key={newestIncompleteMatch.key}


I am not sure why you removed this attribute. Having a key attribute helps Preact with diffing, so that it knows the difference between when items were changed, added, reordered, or removed. I think there was some weird edge case where when this page rerendered (due to the newestIncompleteMatch switching), it might have reset the input on the event matches list, or something like that.

I removed it because the MatchDetailsCard component doesn't have key as a property. I don't know what it's passing into.

src/routes/report-edit.tsx

calebeby · 2024-07-05T17:56:34Z

src/routes/saved-report.tsx

+      </Card>
+    </Page>
+  )
+}


Before this PR, ReportPage was shared between the offline report editor and the "online" report editor. It looks like you split it out here. My concern is that the two pages (which are intended to look and work essentially the same) may now get out of sync in case someone forgets to edit both. For example, someone may make a CSS change in one but not the other. The only real difference between the two is how they handle fetching/storing the report data. Is there a way to combine them back together? (same for the viewer and editor I think)

I did what I thought would work well here, but I didn't do a thorough comparison of every pair of files among the six that existed. There isn't enough overlap between the viewer and the editor to consolidate.

changed back on match page to window's back

ec32029

varCepheid self-assigned this Jan 15, 2023

varCepheid added the enhancement Adds a new feature or meets a request. label Jan 15, 2023

varCepheid requested a review from calebeby January 15, 2023 21:07

varCepheid added needs work Changes have been suggested and need to be implemented. priority This needs to be addressed before the current changes can be merged to main. wip and removed needs work Changes have been suggested and need to be implemented. labels Jan 26, 2023

removed back attribute in page

71595ac

varCepheid added the needs work Changes have been suggested and need to be implemented. label Jan 27, 2023

calebeby marked this pull request as draft January 27, 2023 21:46

added a link to event in match details card

2fb4e73

varCepheid removed the wip label Feb 9, 2023

varCepheid added 4 commits February 8, 2023 17:39

Merge branch 'dev' into match-back-button

9abd4c7

more work on event links in matches

4c1b581

moved the link under the name of the match and removed it from the event page

fixed team numbers in match details card

5a142a7

continued trying to fix team numbers

662e43f

varCepheid added 6 commits February 9, 2023 20:25

moved event title above date and removed back attributes

3c7179f

attempted to fix grid layout

1410018

more styling for match cards

4926a33

grid styling, back button fix

5ad746c

color and spacing fixes

b9ba663

one more spacing fix

583597a

varCepheid added question Further information is requested. and removed needs work Changes have been suggested and need to be implemented. labels Feb 10, 2023

changed link colors, added hover

79dff07

varCepheid marked this pull request as ready for review June 25, 2023 23:42

varCepheid requested a review from calebeby June 25, 2023 23:42

varCepheid removed the needs work Changes have been suggested and need to be implemented. label Aug 2, 2023

calebeby requested changes Aug 9, 2023

View reviewed changes

calebeby added the needs work Changes have been suggested and need to be implemented. label Aug 9, 2023

varCepheid added 5 commits November 7, 2023 14:15

moved headerLinkStyle to Page

5f8c4a9

reverted changes to match-card

6e1116b

minor changes

d0642c7

changed homepage argument of Page & Header to showBackButton and flipped the logic changed onEditClick argument of report-viewer to reportEditorLink and changed the Button to href shortened functions in saved-report-edit

more moving event-page header styles to Page

9458155

changed colors greenOnPurple and redOnPurple to match accessibility g…

3f90658

…uidelines

varCepheid added wip and removed needs work Changes have been suggested and need to be implemented. labels Nov 7, 2023

varCepheid removed the wip label Nov 15, 2023

calebeby self-requested a review November 19, 2023 06:37

calebeby and others added 4 commits January 5, 2024 19:19

Merge remote-tracking branch 'origin/dev' into match-back-button

5c613a6

Merge remote-tracking branch 'origin/dev' into match-back-button

6d69601

Merge branch 'dev' into match-back-button

68b8bd4

Remove back prop from about page component

5e2bbc4

calebeby reviewed Feb 10, 2024

View reviewed changes

src/colors.ts Outdated Show resolved Hide resolved

calebeby reviewed Feb 10, 2024

View reviewed changes

src/components/page.tsx Outdated Show resolved Hide resolved

calebeby requested changes Feb 10, 2024

View reviewed changes

requested changes

ee45d65

calebeby self-requested a review July 5, 2024 17:27

calebeby changed the title ~~change back button to last viewed page~~ Change back button to last viewed page Jul 5, 2024

calebeby reviewed Jul 5, 2024

View reviewed changes

consolidated report-page elements, renamed report-edit-link

50d9762

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change back button to last viewed page #1522

Change back button to last viewed page #1522

varCepheid commented Jan 15, 2023 •

edited

Loading

calebeby commented Jan 26, 2023

calebeby commented Jan 26, 2023

varCepheid commented Jan 26, 2023

varCepheid commented Feb 9, 2023

varCepheid commented Feb 10, 2023

calebeby left a comment

varCepheid commented Nov 7, 2023

varCepheid commented Feb 5, 2024

calebeby left a comment

varCepheid commented Feb 17, 2024

calebeby left a comment

calebeby Jul 5, 2024

varCepheid Jul 5, 2024

calebeby Jul 5, 2024 •

edited

Loading

varCepheid Jul 5, 2024

Change back button to last viewed page #1522

Are you sure you want to change the base?

Change back button to last viewed page #1522

Conversation

varCepheid commented Jan 15, 2023 • edited Loading

calebeby commented Jan 26, 2023

calebeby commented Jan 26, 2023

varCepheid commented Jan 26, 2023

varCepheid commented Feb 9, 2023

varCepheid commented Feb 10, 2023

calebeby left a comment

Choose a reason for hiding this comment

varCepheid commented Nov 7, 2023

varCepheid commented Feb 5, 2024

calebeby left a comment

Choose a reason for hiding this comment

varCepheid commented Feb 17, 2024

calebeby left a comment

Choose a reason for hiding this comment

calebeby Jul 5, 2024

Choose a reason for hiding this comment

varCepheid Jul 5, 2024

Choose a reason for hiding this comment

calebeby Jul 5, 2024 • edited Loading

Choose a reason for hiding this comment

varCepheid Jul 5, 2024

Choose a reason for hiding this comment

varCepheid commented Jan 15, 2023 •

edited

Loading

calebeby Jul 5, 2024 •

edited

Loading