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 Log Types #60

Merged
merged 20 commits into from
Jan 23, 2024
Merged

Change Log Types #60

merged 20 commits into from
Jan 23, 2024

Conversation

cpresler
Copy link
Contributor

Added:

  • Displaying change log types (uncommented and enhanced existing structure)
  • Menu for selecting log type that is activated/deactivated by clicking on the default log type (added)
    • Menu indicates currently active type and does not update anything if existing type is chosen
  • Move log type declaration into the types file and explicitly import/exported the related type declarations
  • showTypeMenu added to ChangeLog type as an optional property to avoid breaking existing ChangeLogs that do not include it
  • Property menu includes Log Types toggle for showing/hiding types on all change logs

Testing:

  • create new FigLog - add change logs, modify their types
  • "modify" a change log's type to the current type - should not show as edited, or have the edited time update
  • modify change log types on existing FigLog instances without errors

@cpresler cpresler linked an issue Jan 11, 2024 that may be closed by this pull request
@ryansrofe
Copy link
Collaborator

This is looking great. I love this new feature and we just had someone ask about it in the comments so great timing!

Some thoughts I had...

  • Can we start with the dashed outlined "add type"
    • this will help with older versions as updating will make all logs start with the type added
Screenshot 2024-01-19 at 10 15 59 AM
  • The first time they choose a type maybe we don't update the edited flag? If they update it again we should update the edited flag.

  • If we click on the same log type maybe we close the menu instead of no action?

  • Can we close the log type menu if we click outside it anywhere?

  • If one log is open and we open another can we close the first one so there is only one log type menu open at a time?

@cpresler
Copy link
Contributor Author

  • Can we start with the dashed outlined "add type"
    • this will help with older versions as updating will make all logs start with the type added

I can definitely add the "add type" dashed version to the log types and start out all new logs with that type, but all existing ones already have "added" set as their type - it is included in Widget.tsx line 29 on main

  • The first time they choose a type maybe we don't update the edited flag? If they update it again we should update the edited flag.

Sure, I should be able to make it so that changing from "add type" to something else doesn't trigger an "edited" change.

  • If we click on the same log type maybe we close the menu instead of no action?

Good catch! This is a bug - I intended for it to close the menu.

  • Can we close the log type menu if we click outside it anywhere?

I'll investigate if we can add dynamic onclick functionality to the rest of the widget without it messing with the ability to click on things generally.

  • If one log is open and we open another can we close the first one so there is only one log type menu open at a time?

I'll have to include running through all other logs and setting their showTypeMenu props to false - this could have performance implications with a really large number of logs (like 1000+), but probably that is fine.

@ryansrofe
Copy link
Collaborator

one other ordering change in the property menu if we can, let me know if you think this order feels better and visually looks better with all the blue highlights.

Set Status | Name | Description | Version | Log Types | Branding

@cpresler
Copy link
Contributor Author

I'm fine with changing the property menu order. I'll make that update as well!

@ryansrofe
Copy link
Collaborator

Noticing when the type is add type the type menu has a dashed border, when another option is chosen the border is solid. Can we use the solid border only?

Screenshot 2024-01-23 at 10 13 22 AM Screenshot 2024-01-23 at 10 13 34 AM

@cpresler
Copy link
Contributor Author

Noticing when the type is add type the type menu has a dashed border, when another option is chosen the border is solid. Can we use the solid border only?

Screenshot 2024-01-23 at 10 13 22 AM Screenshot 2024-01-23 at 10 13 34 AM

Should be possible. That is a bug I hadn't noticed. Give me a few minutes to fix it

@ryansrofe
Copy link
Collaborator

Testing results from me. I created three widgets using the currently released widget. Then updated the version to that in this branch. All widgets updated as expected with little to no change in UI other than propertyMenu order. Log types work as expected and older logs start as add type.

FigLogV1
FigLogV2
FigLogV1-2
FigLogV2-2
FigLogV1-3
FigLogV2-3

Comment on lines -16 to +17
const [showBranding, setShowBranding] = useSyncedState('showBradning', true);
const [showBranding, setShowBranding] = useSyncedState('showBranding', true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does changing this, ugh my fat fingers, possibly introduce this setting breaking on older logs?

Copy link
Contributor Author

@cpresler cpresler Jan 23, 2024

Choose a reason for hiding this comment

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

I don't think so, did you try turning the branding on/off for the ones that you created from main? It seemed fine on the ones I checked.
It is one of those things that I was actually confused was working in the first place because you correctly spelled the variable everywhere except that spot.

updateChange({
showTypeMenu: !changeLog.showTypeMenu,
})
// hide all other log type menues
Copy link
Collaborator

Choose a reason for hiding this comment

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

👏 I didn't think we were able to do this without perf issues, thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might have perf issues with a ton (~1000) logs since it loops through and sets state on each one, but there are probably other issues at that point anyhow.

Copy link
Collaborator

@ryansrofe ryansrofe left a comment

Choose a reason for hiding this comment

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

👍

@cpresler cpresler merged commit 59be9f0 into main Jan 23, 2024
1 check passed
@cpresler cpresler deleted the fl15-changelog-type branch January 23, 2024 20:02
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.

Changelog: add ability to choose a changelog type per changelog
2 participants