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.

Sign up for GitHub

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

Jump to bottom

Fixes #1861. Border: Title property is preferable to Text. #1862

Merged
merged 31 commits into from
Aug 1, 2022
Merged

Fixes #1861. Border: Title property is preferable to Text. #1862

merged 31 commits into from
Aug 1, 2022

Conversation

BDisp
Copy link
Collaborator

@BDisp BDisp commented Jul 19, 2022

border-title

BDisp and others added 19 commits July 19, 2022 19:28
@BDisp
Fixes gui-cs#1861. Border Title property is preferable than the Text.
42a3f57
@BDisp
Fixes gui-cs#1866. Bug when scrolling text and type in a TextView. (g…
ad217aa 
…ui-cs#1868)
@BDisp
Some fixes for the WebConsole support. (gui-cs#1865)
c0e5509 
* Invoking NotifyStopRunState for all situations.

* Added Clicked property to support web console.

* Changing to MoveDown to stay always visible.
@tig @BDisp
Fixes gui-cs#1849. Wizard as non-popup is broken (gui-cs#1853)
a73d03a 
* trying to make it work

* Fixes gui-cs#1849. Wizard as non-modal doesn't work

* Fixes gui-cs#1855. Window and Frame content view without the margin frame.

* Fixing layout of non-modal

* WizardSTep is now a FrameView

* Now use Modal = false to set visual style automatically

* Removed Controls as an explicit construct. Now just Add to WizardStep

Co-authored-by: BDisp <[email protected]>
@tznind @BDisp
Update docs with keybindings, global key event and designer (gui-cs#1869
8f529fb 
)

* Added docs on keybinding and global key event

* Added TerminalGuiDesigner to showcases/examples
@tig @BDisp
Regenerated Docs (gui-cs#1870)
d69c5b1
@tig @BDisp
Fixed cancel logic. Title now shows for non-modal. (gui-cs#1871)
c060b21
@BDisp
Fixes gui-cs#1874. API docs on github are broken. (gui-cs#1875)
0d668bf 
* Fixes gui-cs#1874. API docs on github are broken.

* Rebuild with docfx 2.59.3.0 version.
@tig @BDisp
Fixes Wizard cancel logic and updates docs (gui-cs#1878)
0e029a4 
* Fixed cancel logic. Title now shows for non-modal.

* trying to fix docs

* trying to fix docs
@BDisp
Fixes gui-cs#1867. Use Undo and Redo commands with WordWrap enabled. (g…
426baa3 
…ui-cs#1877)
@tig @BDisp
Updated docs; regeneraged docs (gui-cs#1881)
5b87a20
@BDisp
Added a 'Read Only' to the Editor scenario Format menu. (gui-cs#1882)
44f0984
@tig @BDisp
Fixes gui-cs#1883. Toplevel now propogates Loaded & Ready events to c…
1647b34 
…hild Toplevel views.
@tig @BDisp
Updated API doc theme. Added Wizard Sample
5767d3f
@tig @BDisp
Tweaked API docs format and content. Fixed build warnings.
068f53a
@tig @BDisp
Fixes gui-cs#1889. Docs broken after org move.
616f618
@tig @BDisp
Regen API docs
7784770
@tig @BDisp
Fixes readme links to API docs
a0f08fe
@BDisp
Merge branch 'develop' into border-title
8373779
@BDisp BDisp mentioned this pull request Jul 26, 2022
Closed
@BDisp
Copy link
Collaborator Author

BDisp commented Jul 26, 2022

I already have done the merge before but perhaps still having some setting under progress and didn't assumed it.

@tig tig changed the title Fixes #1861. Border Title property is preferable than the Text. Fixes #1861. Border: Title property is preferable to Text. Jul 26, 2022
@tig
Copy link
Collaborator

tig commented Jul 26, 2022 •
edited
Loading

@BDisp did you notice I have a question/comment above in my code review? I think this is a breaking change.

tig
tig requested changes Jul 26, 2022
View reviewed changes
Copy link
Collaborator

@tig tig left a comment

Choose a reason for hiding this comment

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

Please either convince me that changing the method signature from string to ustring isn't a breaking change or fix this so that it's not a breaking change.

Terminal.Gui/Core/Border.cs Outdated Show resolved Hide resolved
@BDisp
Copy link
Collaborator Author

BDisp commented Jul 26, 2022

@BDisp did you notice I have a question/comment above in my code review? I think this is a breaking change.

I can´t see it, where?

@tig
Copy link
Collaborator

tig commented Jul 26, 2022

@BDisp did you notice I have a question/comment above in my code review? I think this is a breaking change.

I can´t see it, where?

image

@BDisp
Copy link
Collaborator Author

BDisp commented Jul 26, 2022

It only appeared latter :-)

tig
tig requested changes Jul 27, 2022
View reviewed changes
Copy link
Collaborator

@tig tig left a comment

Choose a reason for hiding this comment

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

I'm curious if you actually thought switching from string to ustring was really going to be a breaking change or not. I see that you reverted that, but it's not clear to me whether you did it just because it was easier than arguing or you actually agreed it was a breaking change.

I've added some suggestions on API documentation as well. Thanks.

Terminal.Gui/Core/Border.cs Outdated Show resolved Hide resolved
Terminal.Gui/Core/Border.cs Outdated Show resolved Hide resolved
Terminal.Gui/Core/Border.cs Outdated Show resolved Hide resolved
@BDisp
Copy link
Collaborator Author

BDisp commented Jul 28, 2022

@tig, is there any problem with this pr?

@tig
Copy link
Collaborator

tig commented Jul 28, 2022

@tig, is there any problem with this pr?

I asked for changes:

image

@BDisp
Copy link
Collaborator Author

BDisp commented Jul 28, 2022 •
edited
Loading

I asked for changes:

Yes, but I already resolved. Did I do something wrong?

@BDisp
Copy link
Collaborator Author

BDisp commented Jul 28, 2022

I'm curious if you actually thought switching from string to ustring was really going to be a breaking change or not. I see that you reverted that, but it's not clear to me whether you did it just because it was easier than arguing or you actually agreed it was a breaking change.

Sorry, I didn't see this comment before. I agree that will cause a breaking change if the user pass a string variable to the constructor. Passing a literal string wouldn't causing problems. But we must supposed that this class can being used by others users and reverting was the best solution. So, in the Initialize method I cast it to ustring and problem is resolved.

I've added some suggestions on API documentation as well. Thanks.

I think I already did the suggestions changes. Thanks.

@BDisp
Copy link
Collaborator Author

BDisp commented Jul 30, 2022

I don't know if it's a work around for the #1827.

@tig tig merged commit 49e2b95 into gui-cs:develop Aug 1, 2022
@BDisp BDisp deleted the border-title branch August 1, 2022 23:08
@BDisp BDisp mentioned this pull request Aug 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tznind tznind tznind left review comments

@tig tig tig approved these changes

@migueldeicaza migueldeicaza Awaiting requested review from migueldeicaza

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@BDisp @tig @tznind