-
-
Notifications
You must be signed in to change notification settings - Fork 17
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
Added support for "WikiUrl" on duty list items #68
base: main
Are you sure you want to change the base?
Conversation
d2b282a
to
e71d655
Compare
Hi @acapper, thank you for your contribution! I see a lot of structure changes that is probably not needed. The style changes also messes up some things, like the hover state which has unintended black text. No style diff should be necessary for this feature. I spent a few minutes messing around with this and came up with a low-code-diff solution for this new icon: Perhaps you could start from there? The only thing that's needed on top of that would be the conditional stuff you've already implemented, actually using the URL, etc. This is the code in green, in my diff above, feel free to copy-paste:
Once you have a cleaner PR I'm happy taking another look, let me know! :) P.S.: Yeah the db.json file is generated from a spreadsheet I built, which is not in this repo. I'm happy to make the DB changes for you! |
I think I've reworked the entire changelist based on your example. Had to add a bunch of events to different refs which was the one of the reasons I didnt do it before. I think its better now but let me know if you can think of a good solution for the event handlers. |
31faa65
to
27aa855
Compare
Sorry got confused along the way |
I'm just stupid using |
a72f267
to
11837a1
Compare
After thinking about this a little more I think I have a good solution for the mouse over events by tracking and extra bit of state. Had to move the events to the flex container to avoid pixel flicker caused by list item border. Hope you like this better as adding more events wasnt the best approach 😁 |
Added method of displaying information links on duty items. If no "WikiUrl" is found the behaviour should be the same as before.
I believe the information in frontend/src/assets/db.json will also require adjustment but it seems to be generated using scripts/csv_to_json.py which uses csv files that arent included in the repo and I dont see an obvious way to generate them with the available files.